-
Notifications
You must be signed in to change notification settings - Fork 0
feat(005): Smart Repository Filtering #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Feature 005 design artifacts: - spec.md: 3 user stories, 10 functional requirements, 5 success criteria - plan.md: Technical context, constitution check, project structure - research.md: GitHub Search API research and implementation strategy - data-model.md: Entity definitions (ActivityFilterSettings, ActivityStatistics) - contracts/search-api.md: GitHub Search API contract - quickstart.md: Usage guide with examples - tasks.md: 41 tasks organized by user story (TDD approach) - checklists/: Requirements quality validation (52 items) Key decisions: - Hybrid approach: Search API for org repos, client-side for personal - Graceful fallback on rate limit (FR-008) - Uses existing --days parameter for cutoff calculation
Resolve all 52 checklist items in comprehensive.md by enhancing spec.md: - Add glossary defining active/inactive repo, cutoff date, pushed_at - Expand FR-002 with ISO 8601 format and exact filtering logic - Detail FR-003/FR-004 behavior for each menu option [A], [L], [O] - Document FR-006 confirmation prompt options (Y/n/all) - Specify FR-008 fallback steps for rate limit, server errors, timeout - Add FR-010 default behavior when --days not provided - Expand edge cases from 5 to 10 with exact error messages - Clarify SC-001/SC-005 timing measurements - Add SC-006 memory efficiency requirement - Add Performance Requirements table with limits per scenario - Add API Constraints section (rate limits, query limits) - Add Dependencies section with validated internal/external deps - Add Design Decisions section justifying hybrid approach - Quantify time savings in US1/US2 (~80%, ~90%)
Add activity-based filtering to repository selection for [A], [L], [O] options: - Add search_repos() and search_active_org_repos() to GitHubClient - Implement get_cutoff_date(), filter_by_activity(), display_activity_stats() helpers - Modify select_github_repos() with --days parameter and confirmation prompts - Use Search API (org:+pushed:>) for efficient org filtering - Add rate limit fallback and zero-results handling per FR-008/FR-009 Tests: 162 passing (118 unit + 44 feature integration) Tasks: 41/41 completed per tasks.md
Summary of ChangesHello @amargiovanni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the repository selection experience by introducing a 'Smart Repository Filtering' feature. It intelligently filters GitHub repositories based on their recent activity, allowing users to concentrate their analysis on actively maintained projects. The implementation includes efficient integration with the GitHub Search API for organization repositories, clear activity statistics, and user-friendly prompts to manage filtering preferences, alongside robust error handling for various API and data scenarios. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This is an excellent pull request that introduces a valuable smart filtering feature. The changes are comprehensive, covering not only the implementation but also extensive documentation, specifications, and tests. The code is well-structured, clean, and follows existing patterns in the codebase. I've identified a couple of minor areas for improvement to enhance robustness and fully align the implementation with the detailed specifications provided.
| if not active_repos: | ||
| print(f"⚠️ No repositories have been pushed to in the last {days} days.") | ||
| try: | ||
| zero_choice = input("Options: [1] Include all repos, [2] Cancel: ").strip() | ||
| except (EOFError, KeyboardInterrupt): | ||
| return [] | ||
| if zero_choice == "1": | ||
| active_repos = repos | ||
| else: | ||
| continue |
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 handling for zero active repositories is missing the "Adjust timeframe" option that is documented in the feature specification (spec.md, Edge Case #1) and quickstart.md. The current implementation only offers to include all repos or cancel. Adding the option to adjust the timeframe would improve the user experience and align the implementation with the documentation. A more complete implementation could loop to allow the user to enter a new timeframe and re-filter.
| except (EOFError, KeyboardInterrupt, GitHubAnalyzerError): | ||
| pass |
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.
This except block is too broad and silently ignores all GitHubAnalyzerError exceptions that might occur during the fallback process. This could hide important issues, such as authentication problems or other API errors when trying to fetch the unfiltered list of organization repositories. It would be more robust to handle user cancellation (EOFError, KeyboardInterrupt) separately from actual application errors (GitHubAnalyzerError).
| except (EOFError, KeyboardInterrupt, GitHubAnalyzerError): | |
| pass | |
| except (EOFError, KeyboardInterrupt): | |
| # User cancelled the fallback selection | |
| pass | |
| except GitHubAnalyzerError as e: | |
| # Log the error that occurred during fallback | |
| log(f"Error during fallback: {e.message}", "error") |
Add Callable[[str, str], None] type hint to satisfy mypy.
Summary
org:andpushed:>qualifiers for efficient org filteringChanges
src/github_analyzer/api/client.py: Addedsearch_repos()andsearch_active_org_repos()methodssrc/github_analyzer/cli/main.py: Added filtering helpers (get_cutoff_date,filter_by_activity,display_activity_stats), modifiedselect_github_repos()with--daysparametertests/integration/test_smart_filter.py: 24 new integration teststests/unit/: 20 new unit tests for API and CLI componentsTest plan
pytest tests/ -vconfirms no regressionsruff check)