diff --git a/.github/workflows/points.yml b/.github/workflows/points.yml index efaa924f..77f8816b 100644 --- a/.github/workflows/points.yml +++ b/.github/workflows/points.yml @@ -13,8 +13,9 @@ permissions: jobs: assign-points: runs-on: ubuntu-latest + # Only run for PR reviews or comments on PRs (not regular issues) if: > - (github.event_name == 'pull_request_review') || + github.event_name == 'pull_request_review' || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null) steps: - name: Checkout repository @@ -28,15 +29,16 @@ jobs: uses: actions/setup-python@v5 with: python-version: '3.11' + check-latest: true + cache: 'pip' - name: Install dependencies - run: | - pip install PyYAML + run: pip install PyYAML - - name: Assign points + - name: Run points script id: assign_points run: | - set +e + set +e # Don't exit on error python scripts/assign_points.py exit_code=$? echo "exit_code=$exit_code" >> $GITHUB_OUTPUT @@ -51,20 +53,30 @@ jobs: else exit $exit_code fi - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Update leaderboard + - name: Update leaderboard markdown if: steps.assign_points.outputs.exit_code == '0' - run: | - python scripts/update_leaderboard.py + run: python scripts/update_leaderboard.py - - name: Commit and push leaderboard + - name: Create Pull Request if: steps.assign_points.outputs.exit_code == '0' - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add leaderboard.json LEADERBOARD.md - git diff --staged --quiet || git commit -m "Update leaderboard [skip ci]" - git pull --rebase origin main - git push origin main + uses: peter-evans/create-pull-request@v6 + with: + token: ${{ secrets.GITHUB_TOKEN }} + add: 'leaderboard.json,LEADERBOARD.md' + commit-message: "Update leaderboard" + branch: leaderboard-update-${{ github.run_id }} + delete-branch: true + title: "Update contributor leaderboard" + body: | + ## Leaderboard Update + + This PR updates the contributor leaderboard based on recent PR review activity. + + **Triggered by:** ${{ github.event_name }} + **Run:** ${{ github.run_number }} + + Please review and merge to update the leaderboard. + labels: | + leaderboard + automated \ No newline at end of file diff --git a/scripts/assign_points.py b/scripts/assign_points.py index 5ba6bb7b..1bb60045 100644 --- a/scripts/assign_points.py +++ b/scripts/assign_points.py @@ -1,5 +1,4 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. +# Copyright (c) Microsoft. All rights reserved. import os import json @@ -17,11 +16,6 @@ # Path to leaderboard in repository root (one level up from scripts/) LEADERBOARD_FILE = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'leaderboard.json') -# Exit codes -EXIT_SUCCESS = 0 -EXIT_ERROR = 1 -EXIT_NO_OP = 2 # No changes needed - def load_config(): """ Load the points configuration from config_points.yml. @@ -41,7 +35,7 @@ def load_config(): with open(CONFIG_FILE, 'r', encoding='utf-8') as f: config = yaml.safe_load(f) except yaml.YAMLError as e: - print(f"ERROR: Invalid YAML in config file: {e}", file=sys.stderr) + print(f"ERROR: Invalid YAML syntax in config file: {e}", file=sys.stderr) print(f"File location: {CONFIG_FILE}", file=sys.stderr) sys.exit(1) except Exception as e: @@ -51,26 +45,7 @@ def load_config(): # Validate that config has the expected structure if not isinstance(config, dict) or 'points' not in config: print(f"ERROR: Invalid config structure in {CONFIG_FILE}", file=sys.stderr) - print("Expected format: { points: { basic_review: 5, ... } }", file=sys.stderr) - sys.exit(1) - - # Validate required keys - required_keys = ['basic_review', 'detailed_review', 'performance_improvement', 'approve_pr'] - missing_keys = [key for key in required_keys if key not in config['points']] - if missing_keys: - print(f"ERROR: Missing required keys in config: {', '.join(missing_keys)}", file=sys.stderr) - print(f"Required keys: {', '.join(required_keys)}", file=sys.stderr) - sys.exit(1) - - # Validate that all required point values are positive integers - invalid_keys = [ - key for key in required_keys - if not isinstance(config['points'][key], int) or config['points'][key] <= 0 - ] - if invalid_keys: - print(f"ERROR: The following point values are not positive integers: {', '.join(invalid_keys)}", file=sys.stderr) - for key in invalid_keys: - print(f" {key}: {config['points'][key]!r}", file=sys.stderr) + print("Expected format: { points: { review_submission: 5, detailed_review: 5, approve_pr: 3, pr_comment: 2 } }", file=sys.stderr) sys.exit(1) return config @@ -91,34 +66,35 @@ def load_event(): print(f"ERROR: Invalid JSON in event file: {e}", file=sys.stderr) print(f"File location: {event_path}", file=sys.stderr) sys.exit(1) - except Exception as e: - print(f"ERROR: Failed to read event file: {e}", file=sys.stderr) - sys.exit(1) - - # Check if this is a comment on a regular issue (not a PR) + + # Validate that this is a PR-related event, not a regular issue comment if 'issue' in event and 'pull_request' not in event.get('issue', {}): print("INFO: Skipping - this is a comment on a regular issue, not a pull request.") - sys.exit(EXIT_NO_OP) - + sys.exit(2) # Exit code 2 = no-op + return event def load_processed_ids(): if os.path.exists(PROCESSED_FILE): with open(PROCESSED_FILE, 'r', encoding='utf-8') as f: try: - # Load as list, convert to set for efficient lookup return set(json.load(f)) except json.JSONDecodeError: return set() return set() -def save_processed_ids(processed_ids): +def save_processed_ids(ids): + """ + Save processed event IDs to prevent duplicate scoring. + + This is critical for data integrity - if this fails after points + are awarded, the same event could be scored multiple times on retry. + """ try: with open(PROCESSED_FILE, 'w', encoding='utf-8') as f: - # Convert set to list for JSON serialization - json.dump(list(processed_ids), f, indent=2) + json.dump(list(ids), f, indent=2) except PermissionError as e: - print(f"ERROR: Permission denied writing to {PROCESSED_FILE}: {e}", file=sys.stderr) + print(f"ERROR: Permission denied when saving processed IDs to {PROCESSED_FILE}: {e}", file=sys.stderr) print("Check file permissions and ensure the workflow has write access.", file=sys.stderr) sys.exit(1) except IOError as e: @@ -150,7 +126,7 @@ def extract_user(event): if login: return login, 'review.user' - # Try comment user + # Try comment user second comment = event.get('comment') if comment and isinstance(comment, dict): comment_user = comment.get('user') @@ -159,59 +135,41 @@ def extract_user(event): if login: return login, 'comment.user' - # Fallback to sender + # Fallback to top-level sender (most reliable) sender = event.get('sender') if sender and isinstance(sender, dict): login = sender.get('login') if login: return login, 'sender' + # All extraction methods failed return None, None def detect_points(event, cfg): """ - Detect points based on the GitHub event and configuration. - - All keyword matching is CASE-INSENSITIVE. Contributors can use any capitalization. + Calculate points for a GitHub event based on review actions. Scoring Rules: - 1. Review types (mutually exclusive - only the highest applies): - - Include "detailed" anywhere in your review = detailed_review points (10) - - Include "basic review" anywhere in your review = basic_review points (5) - - If both keywords present, only "detailed" counts (higher value) - - 2. Bonus points (additive - can stack with review types): - - Include "performance" anywhere = performance_improvement bonus (+4) - - Approve the PR (state=approved) = approve_pr bonus (+3) - - Keyword Examples (all case-insensitive): - - "detailed", "Detailed", "DETAILED" all work - - "basic review", "Basic Review", "BASIC REVIEW" all work - - "performance", "Performance", "PERFORMANCE" all work - - Args: - event: GitHub webhook event JSON - cfg: Configuration dictionary with points values - - Returns: - tuple: (points: int, user: str) + 1. Any PR review submission = review_submission points (base points) + 2. PR approval (state=approved) = approve_pr bonus (additive) + 3. Substantial review (comment length >= 100 characters) = detailed_review bonus (additive) - Exits: - 1 if unable to extract user from event - 2 if no points criteria matched (not an error) + Scoring Examples: + - Simple review with short comment = 5 points (base) + - Review with detailed feedback (100+ chars) = 5 + 5 = 10 points + - Approved PR = 5 + 3 = 8 points + - Approved PR with detailed feedback = 5 + 3 + 5 = 13 points + - Comment on PR (not a review) = 2 points """ - action = event.get('action') - review = event.get('review', {}) - comment = event.get('comment', {}) + action = event.get('action', '') + review = event.get('review') or {} + comment = event.get('comment') or {} - # Note: The review and comment bodies are user-controlled content from the GitHub API. - # No sanitization is performed here because the current usage only checks for keywords. - # If this code is extended to process or display these bodies, input validation should be added. - review_body = (review.get('body') or '').lower() + review_body = review.get('body') or '' review_state = (review.get('state') or '').lower() - comment_body = (comment.get('body') or '').lower() user, source = extract_user(event) + if not user: print("ERROR: Unable to extract user from event. Checked review.user, comment.user, and sender fields.") print("Event structure:", json.dumps({ @@ -226,24 +184,30 @@ def detect_points(event, cfg): points = 0 scoring_breakdown = [] - - # Review type scoring (mutually exclusive - highest wins) - if "detailed" in review_body: - points += cfg['points']['detailed_review'] - scoring_breakdown.append(f"detailed_review: +{cfg['points']['detailed_review']}") - elif "basic review" in review_body: - points += cfg['points']['basic_review'] - scoring_breakdown.append(f"basic_review: +{cfg['points']['basic_review']}") - # Performance improvement bonus (additive) - if "performance" in comment_body or "performance" in review_body: - points += cfg['points']['performance_improvement'] - scoring_breakdown.append(f"performance_improvement: +{cfg['points']['performance_improvement']}") + # Determine if this is a review or just a comment + is_review = action == "submitted" and event.get('review') is not None and event.get('review') + is_comment = event.get('comment') is not None and event.get('comment') and not is_review - # PR approval bonus (additive) - if action == "submitted" and review_state == "approved": - points += cfg['points']['approve_pr'] - scoring_breakdown.append(f"approve_pr: +{cfg['points']['approve_pr']}") + if is_review: + # Base points for any PR review submission + points += cfg['points']['review_submission'] + scoring_breakdown.append(f"review_submission: +{cfg['points']['review_submission']}") + + # Bonus for substantial review (100+ characters) + if len(review_body.strip()) >= 100: + points += cfg['points']['detailed_review'] + scoring_breakdown.append(f"detailed_review: +{cfg['points']['detailed_review']}") + + # Bonus for approving the PR + if review_state == "approved": + points += cfg['points']['approve_pr'] + scoring_breakdown.append(f"approve_pr: +{cfg['points']['approve_pr']}") + + elif is_comment: + # Points for commenting on a PR (less than review) + points += cfg['points']['pr_comment'] + scoring_breakdown.append(f"pr_comment: +{cfg['points']['pr_comment']}") # Log scoring breakdown for transparency if scoring_breakdown: @@ -266,20 +230,17 @@ def update_leaderboard(user, points): if os.path.exists(LEADERBOARD_FILE): with open(LEADERBOARD_FILE, 'r', encoding='utf-8') as f: try: - data = json.load(f) - # Filter out 'top' key to prevent unbounded growth - leaderboard = {k: v for k, v in data.items() if k != 'top' and isinstance(v, (int, float))} + leaderboard = json.load(f) except json.JSONDecodeError: leaderboard = {} leaderboard[user] = leaderboard.get(user, 0) + points - print(f"Updated {user}: {leaderboard[user]} total points (awarded +{points})") - + try: with open(LEADERBOARD_FILE, 'w', encoding='utf-8') as f: json.dump(leaderboard, f, indent=2) except PermissionError as e: - print(f"ERROR: Permission denied writing to {LEADERBOARD_FILE}: {e}", file=sys.stderr) + print(f"ERROR: Permission denied when saving leaderboard to {LEADERBOARD_FILE}: {e}", file=sys.stderr) print("Check file permissions and ensure the workflow has write access.", file=sys.stderr) sys.exit(1) except IOError as e: @@ -296,28 +257,26 @@ def main(): points, user = detect_points(event, cfg) # Extract unique ID for duplicate prevention - event_id = event.get('review', {}).get('id') - if event_id is None: - event_id = event.get('comment', {}).get('id') - if event_id is None: + event_id = event.get('review', {}).get('id') or event.get('comment', {}).get('id') + if not event_id: print("No unique ID found in event. Skipping duplicate check.") - sys.exit(EXIT_NO_OP) + sys.exit(2) # Exit code 2 = no-op (not an error) processed_ids = load_processed_ids() if event_id in processed_ids: print(f"Event {event_id} already processed. Skipping scoring.") - sys.exit(EXIT_NO_OP) + sys.exit(2) # Exit code 2 = no-op (not an error) if points <= 0: - print("No points awarded. Skipping leaderboard update.") - sys.exit(EXIT_NO_OP) + print("No points awarded for this event.") + sys.exit(2) # Exit code 2 = no-op (not an error) - # Update leaderboard and mark event as processed + # Update leaderboard first, then mark as processed + # This order ensures we can retry if processed_ids save fails update_leaderboard(user, points) processed_ids.add(event_id) save_processed_ids(processed_ids) - - print(f"SUCCESS: Awarded {points} points to {user}") + sys.exit(0) # Exit code 0 = success (points awarded) -if __name__ == '__main__': - main() +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/scripts/config_points.yml b/scripts/config_points.yml index ef0c04d9..627bf51f 100644 --- a/scripts/config_points.yml +++ b/scripts/config_points.yml @@ -1,25 +1,23 @@ # Contributor Points Configuration # -# All keyword matching is CASE-INSENSITIVE. Contributors can use any capitalization. -# For example: "detailed", "Detailed", "DETAILED" all work the same way. +# Scoring is based on actual review actions, not keywords. +# All contributions are valued and tracked automatically. # # Scoring Rules: -# 1. Review types are mutually exclusive (only highest value applies) -# - "detailed" keyword awards detailed_review points -# - "basic review" keyword awards basic_review points (only if "detailed" not present) -# 2. Bonuses are additive and can stack with review types: -# - "performance" keyword adds performance_improvement bonus -# - Approved PR (state=approved) adds approve_pr bonus +# 1. Any PR review submission = review_submission points (base points) +# 2. Substantial review (100+ characters, excluding whitespace) = detailed_review bonus (additive) +# 3. PR approval (state=approved) = approve_pr bonus (additive) +# 4. PR comment (not a full review) = pr_comment points # -# Examples (all case-insensitive): -# - "basic review" OR "Basic Review" OR "BASIC REVIEW" = 5 points -# - "detailed analysis" OR "Detailed Analysis" = 10 points -# - "detailed performance review" OR "DETAILED PERFORMANCE REVIEW" = 10 + 4 = 14 points -# - Approved PR with "basic review" = 5 + 3 = 8 points -# - Approved PR with "detailed performance review" = 10 + 4 + 3 = 17 points +# Examples: +# - Simple review with short comment = 5 points +# - Review with detailed comment (100+ characters) = 5 + 5 = 10 points +# - Approved PR = 5 + 3 = 8 points +# - Approved PR with detailed feedback = 5 + 5 + 3 = 13 points +# - Comment on PR (not a review) = 2 points points: - basic_review: 5 # Simple review with "basic review" keyword (case-insensitive) - detailed_review: 10 # In-depth review with "detailed" keyword (case-insensitive, overrides basic) - performance_improvement: 4 # Bonus for mentioning "performance" (case-insensitive, additive) - approve_pr: 3 # Bonus for approving a PR (additive) + review_submission: 5 # Base points for submitting any PR review + detailed_review: 5 # Bonus for substantial review (100+ characters of feedback) + approve_pr: 3 # Bonus for approving a PR + pr_comment: 2 # Points for commenting on a PR (not a full review) \ No newline at end of file