diff --git a/.github/workflows/good-egg.yml b/.github/workflows/good-egg.yml index fb3c08f..0e7d482 100644 --- a/.github/workflows/good-egg.yml +++ b/.github/workflows/good-egg.yml @@ -11,6 +11,7 @@ jobs: score: runs-on: ubuntu-latest steps: - - uses: 2ndSetAI/good-egg@v0 + - uses: 2ndSetAI/good-egg@better-egg with: github-token: ${{ secrets.GITHUB_TOKEN }} + scoring-model: v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index 7774860..66574e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.0.0] - 2026-02-23 + +### Added + +- **Better Egg (v2) scoring model** -- opt-in combined model that extends the + graph score with merge rate and account age features via logistic regression. + Trained on 5,129 PRs (of 5,417 total, filtered to those with merge rate + data) from 49 repositories. +- `scoring_model` config option (`v1` default, `v2` for Better Egg). +- `v2:` config block with `graph:`, `features:`, and `combined_model:` + sub-sections. +- `--scoring-model` CLI option. +- `scoring-model` GitHub Action input and output. +- `scoring_model` MCP server parameter on scoring tools. +- `GOOD_EGG_SCORING_MODEL` environment variable override. +- Component score breakdown in v2 output (`graph_score`, `merge_rate`, + `log_account_age`). +- `scoring_model` and `component_scores` fields on `TrustScore` model. +- "Better Egg" branding on PR comments when using v2. +- Example workflow for v2: `examples/better-egg-workflow.yml`. + ## [0.1.0] - 2026-02-10 ### Added @@ -22,4 +43,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - YAML configuration with environment variable overrides. - Multiple output formatters: Markdown, CLI table, JSON, and GitHub check-run. +[1.0.0]: https://github.com/2ndSetAI/good-egg/releases/tag/v1.0.0 [0.1.0]: https://github.com/2ndSetAI/good-egg/releases/tag/v0.1.0 diff --git a/README.md b/README.md index 96bd498..ab2faab 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,20 @@ Add to Claude Desktop (`claude_desktop_config.json`): See [docs/mcp-server.md](https://github.com/2ndSetAI/good-egg/blob/main/docs/mcp-server.md) for tool reference. +## Scoring Models + +Good Egg supports two scoring models: + +| Model | Name | Description | +|-------|------|-------------| +| `v1` | Good Egg (default) | Graph-based scoring from contribution history | +| `v2` | Better Egg | Graph score + merge rate + account age via logistic regression | + +To use v2, set `scoring_model: v2` in your `.good-egg.yml`, pass +`--scoring-model v2` on the CLI, or set `scoring-model: v2` in the action +input. See [Methodology](https://github.com/2ndSetAI/good-egg/blob/main/docs/methodology.md#better-egg-v2) for how the +v2 model works. + ## How It Works Good Egg builds a weighted contribution graph from a user's merged PRs and diff --git a/action.yml b/action.yml index f7a8dad..68a4f3e 100644 --- a/action.yml +++ b/action.yml @@ -22,6 +22,9 @@ inputs: description: 'Fail the action if trust level is LOW' required: false default: 'false' + scoring-model: + description: 'Scoring model to use (v1 or v2)' + required: false outputs: score: @@ -33,6 +36,9 @@ outputs: user: description: 'GitHub username that was scored' value: ${{ steps.score.outputs.user }} + scoring-model: + description: 'Scoring model that was used (v1 or v2)' + value: ${{ steps.score.outputs.scoring-model }} runs: using: 'composite' @@ -62,6 +68,7 @@ runs: INPUT_COMMENT: ${{ inputs.comment }} INPUT_CHECK-RUN: ${{ inputs.check-run }} INPUT_FAIL-ON-LOW: ${{ inputs.fail-on-low }} + INPUT_SCORING_MODEL: ${{ inputs.scoring-model }} run: | cd ${{ github.action_path }} uv run python -m good_egg.action diff --git a/docs/configuration.md b/docs/configuration.md index a7d42d7..84a7ceb 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -25,9 +25,26 @@ Configuration values are resolved in this order (highest priority first): 3. **YAML config file** 4. **Built-in defaults** +## Scoring Model + +Good Egg supports two scoring models. Set the model at the top level of the +config file: + +```yaml +scoring_model: v1 # default -- graph-based scoring only +scoring_model: v2 # Better Egg -- graph + external features via logistic regression +``` + +When using v2, PR comments are branded "Better Egg" instead of "Good Egg". +See [methodology.md](methodology.md#better-egg-v2) for how the v2 model +works. + ## Full YAML Schema ```yaml +# Scoring model selection: v1 (default) or v2 +scoring_model: v1 + # Graph-based scoring algorithm parameters graph_scoring: alpha: 0.85 # Damping factor (0-1) @@ -82,10 +99,53 @@ language_normalization: Go: 2.30 Rust: 2.63 # ... see examples/.good-egg.yml for the full list + +# v2 (Better Egg) scoring model parameters +# Only used when scoring_model is set to v2. +v2: + graph: + half_life_days: 180 # Recency decay half-life for v2 graph + max_age_days: 730 # Ignore PRs older than this + archived_penalty: 0.5 # Penalty multiplier for archived repos + fork_penalty: 0.3 # Penalty multiplier for forked repos + features: + merge_rate: true # Include merge rate feature + account_age: true # Include account age feature + combined_model: + intercept: -0.8094 # Logistic regression intercept + graph_score_weight: 1.9138 # Weight for graph score + merge_rate_weight: -0.7783 # Weight for merge rate + account_age_weight: 0.1493 # Weight for log(account_age_days + 1) ``` ## Config Sections +### scoring_model + +Selects the scoring model. Set to `v1` (default) for graph-only scoring or +`v2` for the Better Egg combined model. When set to `v2`, the parameters +under the `v2:` block are used and the graph construction is simplified +(no self-contribution penalty, no language normalization in repo quality, no +diversity/volume adjustment). Language match personalization weighting +(`same_language_weight`) is retained in v2. + +### v2 (Better Egg) + +Configuration for the Better Egg (v2) scoring model. This section is only +used when `scoring_model` is set to `v2`. + +- **`v2.graph`** -- Graph construction parameters for the simplified v2 graph. + Supports `half_life_days`, `max_age_days`, `archived_penalty`, and + `fork_penalty`. Note that v2 shares graph algorithm parameters (`alpha`, + `max_iterations`, `tolerance`) with the top-level `graph_scoring` config. +- **`v2.features`** -- Toggle external features on or off. `merge_rate` + (merged/(merged+closed)) and `account_age` (log-transformed days) are both + enabled by default. +- **`v2.combined_model`** -- Logistic regression coefficients. The final + score is `sigmoid(intercept + graph_score_weight * graph_score + + merge_rate_weight * merge_rate + account_age_weight * + log(account_age_days + 1))`. + ### graph_scoring Controls the graph-based scoring algorithm. The `alpha` parameter is the @@ -143,6 +203,7 @@ The following environment variables override individual config values: | `GOOD_EGG_HIGH_TRUST` | `thresholds.high_trust` | float | | `GOOD_EGG_MEDIUM_TRUST` | `thresholds.medium_trust` | float | | `GOOD_EGG_HALF_LIFE_DAYS` | `recency.half_life_days` | int | +| `GOOD_EGG_SCORING_MODEL` | `scoring_model` | str (`v1` or `v2`) | ## Programmatic Configuration @@ -167,5 +228,5 @@ config = load_config(".good-egg.yml") The `GoodEggConfig` class is composed of the following sub-configs: `GraphScoringConfig`, `EdgeWeightConfig`, `RecencyConfig`, -`ThresholdConfig`, `CacheTTLConfig`, `LanguageNormalization`, and -`FetchConfig`. +`ThresholdConfig`, `CacheTTLConfig`, `LanguageNormalization`, +`FetchConfig`, and (for v2) `V2Config`. diff --git a/docs/github-action.md b/docs/github-action.md index 3f253a4..0431201 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -37,6 +37,7 @@ This posts a trust score comment on each pull request: | `comment` | No | `true` | Post a PR comment with the trust score | | `check-run` | No | `false` | Create a check run with the trust score | | `fail-on-low` | No | `false` | Fail the action if trust level is LOW | +| `scoring-model` | No | `v1` | Scoring model: `v1` (Good Egg) or `v2` (Better Egg) | ## Outputs @@ -45,6 +46,7 @@ This posts a trust score comment on each pull request: | `score` | Normalized trust score (0.0 - 1.0) | | `trust-level` | Trust level: HIGH, MEDIUM, LOW, UNKNOWN, or BOT | | `user` | GitHub username that was scored | +| `scoring-model` | Scoring model used: `v1` (Good Egg) or `v2` (Better Egg) | ## Custom Configuration @@ -151,6 +153,28 @@ PRs and repository metadata. To stay within rate limits: - The built-in cache (SQLite-backed) avoids refetching data that has not changed. Cache TTLs are configurable. +## Using Better Egg (v2) + +To use the v2 scoring model, set the `scoring-model` input: + +```yaml +jobs: + score: + runs-on: ubuntu-latest + steps: + - uses: 2ndSetAI/good-egg@v0 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + scoring-model: v2 +``` + +When using v2, PR comments are branded "Better Egg" and include a component +score breakdown showing graph score, merge rate, and account age +contributions. The `scoring-model` output reflects which model was used. + +You can also set `scoring-model` via the `GOOD_EGG_SCORING_MODEL` environment +variable, but the input takes precedence. + ## Example Workflows See the [examples/](../examples/) directory for complete workflow files: @@ -159,3 +183,5 @@ See the [examples/](../examples/) directory for complete workflow files: that posts a PR comment - [strict-workflow.yml](../examples/strict-workflow.yml) -- comment, check run, and fail-on-low +- [better-egg-workflow.yml](../examples/better-egg-workflow.yml) -- v2 + scoring model with component breakdown diff --git a/docs/library.md b/docs/library.md index cd15259..a5a9e23 100644 --- a/docs/library.md +++ b/docs/library.md @@ -86,6 +86,44 @@ result = await score_pr_author( ) ``` +### v2 (Better Egg) Configuration + +To use the v2 scoring model, set `scoring_model` on the config: + +```python +from good_egg import GoodEggConfig, score_pr_author + +config = GoodEggConfig( + scoring_model="v2", + v2={ + "graph": {"half_life_days": 180, "max_age_days": 730}, + "features": {"merge_rate": True, "account_age": True}, + "combined_model": { + "intercept": -0.8094, + "graph_score_weight": 1.9138, + "merge_rate_weight": -0.7783, + "account_age_weight": 0.1493, + }, + }, +) + +result = await score_pr_author( + login="octocat", + repo_owner="octocat", + repo_name="Hello-World", + config=config, +) + +# v2 results include component scores +if result.component_scores: + print(f"Graph score: {result.component_scores['graph_score']:.3f}") + print(f"Merge rate: {result.component_scores['merge_rate']:.3f}") + print(f"Log account age: {result.component_scores['log_account_age']:.3f}") + print(f"Normalized score: {result.normalized_score:.3f}") + +print(f"Scoring model: {result.scoring_model}") +``` + You can also load configuration from a YAML file: ```python @@ -120,6 +158,8 @@ the following fields: | `top_contributions` | `list[ContributionSummary]` | Top repositories contributed to | | `language_match` | `bool` | Whether the user's top language matches the context repo | | `flags` | `dict[str, bool]` | Flags (is_bot, is_new_account, etc.) | +| `scoring_model` | `str` | Scoring model used: `v1` or `v2` | +| `component_scores` | `dict[str, float] \| None` | Component breakdown (v2 only): `graph_score`, `merge_rate`, `log_account_age` | | `scoring_metadata` | `dict[str, Any]` | Internal scoring details | `TrustScore` is a Pydantic model, so you can serialize it: diff --git a/docs/mcp-server.md b/docs/mcp-server.md index d008949..5c7b8f1 100644 --- a/docs/mcp-server.md +++ b/docs/mcp-server.md @@ -82,11 +82,14 @@ Returns the full trust score as JSON, including all fields from the |------|------|----------|-------------| | `username` | `string` | Yes | GitHub username to score | | `repo` | `string` | Yes | Target repository in `owner/repo` format | +| `scoring_model` | `string` | No | Scoring model: `v1` (Good Egg, default) or `v2` (Better Egg) | **Returns:** Full `TrustScore` JSON with all fields (user_login, context_repo, raw_score, normalized_score, trust_level, percentile, account_age_days, total_merged_prs, unique_repos_contributed, -top_contributions, language_match, flags, scoring_metadata). +top_contributions, language_match, flags, scoring_model, component_scores, +scoring_metadata). When `scoring_model` is `v2`, the response includes +`component_scores` with graph_score, merge_rate, and log_account_age. ### check_pr_author @@ -98,8 +101,9 @@ Returns a compact summary suitable for quick checks. |------|------|----------|-------------| | `username` | `string` | Yes | GitHub username to check | | `repo` | `string` | Yes | Target repository in `owner/repo` format | +| `scoring_model` | `string` | No | Scoring model: `v1` (Good Egg, default) or `v2` (Better Egg) | -**Returns:** +**Returns (v1):** ```json { @@ -110,6 +114,23 @@ Returns a compact summary suitable for quick checks. } ``` +**Returns (v2):** + +```json +{ + "user_login": "octocat", + "trust_level": "HIGH", + "normalized_score": 0.82, + "total_merged_prs": 47, + "scoring_model": "v2", + "component_scores": { + "graph_score": 0.78, + "merge_rate": 0.91, + "log_account_age": 3.45 + } +} +``` + ### get_trust_details Returns an expanded breakdown with contributions, flags, and metadata. @@ -120,8 +141,38 @@ Returns an expanded breakdown with contributions, flags, and metadata. |------|------|----------|-------------| | `username` | `string` | Yes | GitHub username to analyse | | `repo` | `string` | Yes | Target repository in `owner/repo` format | +| `scoring_model` | `string` | No | Scoring model: `v1` (Good Egg, default) or `v2` (Better Egg) | -**Returns:** +**Returns (v1):** + +```json +{ + "user_login": "octocat", + "context_repo": "octocat/Hello-World", + "trust_level": "HIGH", + "normalized_score": 0.82, + "raw_score": 0.0045, + "account_age_days": 3650, + "total_merged_prs": 47, + "unique_repos_contributed": 12, + "language_match": true, + "top_contributions": [ + { + "repo_name": "octocat/Hello-World", + "pr_count": 15, + "language": "Python", + "stars": 1200 + } + ], + "flags": { + "is_bot": false, + "is_new_account": false + }, + "scoring_metadata": {} +} +``` + +**Returns (v2):** ```json { @@ -146,6 +197,12 @@ Returns an expanded breakdown with contributions, flags, and metadata. "is_bot": false, "is_new_account": false }, + "scoring_model": "v2", + "component_scores": { + "graph_score": 0.78, + "merge_rate": 0.91, + "log_account_age": 3.45 + }, "scoring_metadata": {} } ``` diff --git a/docs/methodology.md b/docs/methodology.md index 52bbc4e..b5c36a0 100644 --- a/docs/methodology.md +++ b/docs/methodology.md @@ -145,3 +145,156 @@ GitHub rate limits bound how much data can be fetched per user. Good Egg is desi - **Review and issue activity**: PRs aren't the only signal. Code reviews and issue participation indicate engagement patterns. - **Organization membership**: Membership in established GitHub organizations as an additional trust signal. - **Cross-platform data**: Contribution data from GitLab, Codeberg, and other forges to build a more complete picture. + +--- + +## Better Egg (v2) + +The v2 scoring model -- branded "Better Egg" in PR comments -- extends the +graph-based approach with external features combined via logistic regression. +It is opt-in via `scoring_model: v2` in configuration; v1 remains the default. + +### Motivation + +The v1 graph score relies entirely on contribution-graph structure. Two +observable signals sit outside that graph: + +1. **Merge rate** -- the fraction of a user's PRs that were merged vs closed. + The v1 data pipeline only fetches *merged* PRs, creating survivorship bias. + Merge rate re-introduces the rejected-PR signal. +2. **Account age** -- how long the GitHub account has existed. Older accounts + correlate with established contributors and help with cold-start cases + where few merged PRs are available. + +A third candidate, **text dissimilarity** (comparing PR descriptions to +repository README content), was investigated and intentionally excluded. +The signal was inverted -- higher similarity correlated with *lower* merge +probability -- likely because low-effort PRs tend to parrot project language +while experienced contributors write more targeted descriptions. Until this +inverted signal is better understood, it is not included in the production +scoring model. + +### Simplified Graph + +The v2 model uses a simplified graph construction compared to v1: + +- **No self-contribution penalty** -- PRs to your own repos are weighted + equally. +- **No language normalization in repo quality** -- star counts are used + directly without ecosystem-size multipliers. +- **No diversity/volume adjustment** -- the "other repos" weight is static, + not dynamically adjusted. + +Language match personalization weighting (`same_language_weight`) is +**retained** -- both repo quality and language match showed small but +statistically significant effects in the validation study. + +These simplifications reduce the number of tunable parameters and let the +logistic regression handle signal combination instead. + +### External Features + +| Feature | Formula | Range | +|---------|---------|-------| +| Merge rate | `merged / (merged + closed)` | 0.0 -- 1.0 | +| Account age | `log(account_age_days + 1)` | 0.0 -- ~4.3 | + +Both features are computed from data already available through the GitHub API +at no additional cost. + +> **Note on merge rate scope:** The merge rate uses total lifetime counts +> (all merged PRs divided by all merged + closed PRs), not temporally-scoped +> counts limited to a recent window. While temporal scoping was considered in +> the original design, the implementation uses lifetime totals as a deliberate +> simplification -- lifetime counts are cheaper to compute, available from the +> existing data pipeline, and showed sufficient discriminative power in the +> validation study. + +### Combined Model + +The three components -- graph score, merge rate, and log account age -- are +combined via logistic regression: + +``` +p = sigmoid(intercept + w1 * graph_score + w2 * merge_rate + w3 * log_account_age) +``` + +The sigmoid function maps the linear combination to a 0-1 probability, which +is used as the final normalized score. The trained weights are: + +| Coefficient | Value | +|-------------|-------| +| `intercept` | -0.8094 | +| `graph_score_weight` | 1.9138 | +| `merge_rate_weight` | -0.7783 | +| `account_age_weight` | 0.1493 | + +These were trained on the validation study data (see below). + +> **Why is `merge_rate_weight` negative?** The negative coefficient does not +> mean "high merge rate is bad." In a multivariate model, each coefficient is +> conditional on the other features. The graph score already captures +> contribution success history -- users with many merged PRs across repos +> naturally get high graph scores. The negative merge rate weight acts as a +> correction for survivorship bias: users who contribute to many repos +> (reflected in their graph score) tend to attempt more ambitious +> cross-project contributions and consequently have lower merge rates. Given +> an already-high graph score, a very high merge rate does not add further +> positive signal -- the model interprets it as slight evidence of narrower +> contribution scope. + +> **Reduced model for missing merge rate:** When merge rate is unavailable +> (e.g. a user with no prior closed PRs), the model drops the merge rate +> term, effectively using a two-feature reduced model (graph_score + +> account_age). Since the weights were trained with all three features +> present, the reduced model is an approximation -- the coefficients are not +> re-estimated for the two-feature case. + +### Validation + +The v2 model was trained and evaluated on 5,129 PRs (of 5,417 total, +filtered to those with merge rate data) drawn from 49 repositories in a +validation study. + +| Metric | v1 / Good Egg (graph only) | v2 / Better Egg (combined) | +|--------|-----------------|---------------| +| AUC | 0.647 | 0.647 | + +The AUC difference is not statistically significant -- the combined model +does not improve ranking performance over the graph alone. However, the +individual features carry statistically significant information: + +- **Merge rate**: Likelihood Ratio Test p < 10^-12 +- **Account age**: Likelihood Ratio Test p = 1.2 x 10^-5 + +### Why Include Features That Don't Improve AUC? + +The merge rate and account age features are retained despite the flat AUC +because they address structural limitations of the graph-only model: + +- **Merge rate** corrects survivorship bias. The v1 pipeline only sees merged + PRs, so a user with 10 merged and 90 closed looks identical to one with 10 + merged and 0 closed. Merge rate distinguishes them. +- **Account age** provides signal in cold-start scenarios where the graph has + few edges. A 10-year-old account with 2 merged PRs is qualitatively + different from a 2-day-old account with 2 merged PRs. + +Both features carry real information (confirmed by likelihood ratio tests); +the AUC flatness reflects the fact that graph structure already captures most +of the ranking signal in the validation dataset. + +### Component Score Breakdown + +v2 output includes a component-level breakdown showing the individual +contribution of each feature: + +| Component | Description | +|-----------|-------------| +| `graph_score` | Normalized graph-based trust score (same as v1 output) | +| `merge_rate` | Fraction of PRs that were merged (omitted when unavailable) | +| `log_account_age` | Log-transformed account age in days | + +The final logistic regression output is available as `normalized_score` on +the `TrustScore` result. The component scores show the individual feature +values that fed into the combined model, letting users understand which +factors are driving the overall score. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 383de23..b06ed52 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -26,6 +26,30 @@ backoff. If you see persistent failures: | `Could not extract PR number` | Not a PR event | Ensure workflow triggers on `pull_request` | | `Invalid GITHUB_REPOSITORY` | Malformed env var | Check Actions environment | +## v2 (Better Egg) Scoring + +### v2 score differs significantly from v1 + +This is expected. The v2 model uses a simplified graph (no self-contribution +penalty, no language normalization, no diversity/volume adjustment) and +combines the graph score with merge rate and account age via logistic +regression. The final score is calibrated differently and may be higher or +lower than v1 for the same user. + +### Merge rate is 0 or missing + +Merge rate requires both merged and closed PR counts. If the GitHub API +returns no closed PRs (e.g. due to rate limits or data availability), merge +rate defaults to 0. This can lower the combined score. Check that your token +has sufficient rate limit remaining. + +### "Better Egg" appears in PR comments + +PR comments use the "Better Egg" branding when `scoring_model` is set to +`v2`. This is intentional and helps distinguish v2 results from v1. To +switch back, set `scoring_model: v1` or remove the setting (v1 is the +default). + ## Getting Help If you encounter issues not listed here, please [open an issue](https://github.com/2ndSetAI/good-egg/issues). diff --git a/examples/.good-egg.yml b/examples/.good-egg.yml index 48a29a5..f58e56e 100644 --- a/examples/.good-egg.yml +++ b/examples/.good-egg.yml @@ -76,3 +76,37 @@ language_normalization: Zig: 5.44 "F#": 5.57 Nim: 5.96 + +# -------------------------------------------------------------------------- +# v2 (Better Egg) scoring model +# -------------------------------------------------------------------------- +# Uncomment the lines below to enable the v2 combined model. +# See docs/methodology.md for how v2 works. + +# scoring_model: v2 + +# v2: +# # Simplified graph parameters (no self-contribution penalty, +# # no language normalization, no diversity/volume adjustment). +# # Note: v2 shares graph algorithm parameters (alpha, tolerance, etc.) +# # with the top-level graph_scoring config. +# graph: +# half_life_days: 180 # Recency decay half-life +# max_age_days: 730 # Ignore PRs older than this +# archived_penalty: 0.5 # Penalty for archived repos +# fork_penalty: 0.3 # Penalty for forked repos +# +# # External features +# features: +# merge_rate: true # merged / (merged + closed) +# account_age: true # log(account_age_days + 1) +# +# # Logistic regression coefficients +# # score = sigmoid(intercept + graph_score_weight * graph_score +# # + merge_rate_weight * merge_rate +# # + account_age_weight * log(account_age + 1)) +# combined_model: +# intercept: -0.8094 +# graph_score_weight: 1.9138 +# merge_rate_weight: -0.7783 +# account_age_weight: 0.1493 diff --git a/examples/better-egg-workflow.yml b/examples/better-egg-workflow.yml new file mode 100644 index 0000000..8ef5f92 --- /dev/null +++ b/examples/better-egg-workflow.yml @@ -0,0 +1,27 @@ +# Better Egg (v2) workflow -- uses the combined scoring model with +# graph score, merge rate, and account age features. +name: Better Egg + +on: + pull_request: + types: [opened, reopened, synchronize] + +permissions: + pull-requests: write + +jobs: + score: + runs-on: ubuntu-latest + steps: + - id: egg + uses: 2ndSetAI/good-egg@v0 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + scoring-model: v2 + + - name: Print results + run: | + echo "Score: ${{ steps.egg.outputs.score }}" + echo "Trust level: ${{ steps.egg.outputs.trust-level }}" + echo "Model: ${{ steps.egg.outputs.scoring-model }}" + echo "User: ${{ steps.egg.outputs.user }}" diff --git a/scripts/train_v2_weights.py b/scripts/train_v2_weights.py new file mode 100644 index 0000000..e1fd908 --- /dev/null +++ b/scripts/train_v2_weights.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 +"""Train v2 (Better Egg) combined model weights from validation study data. + +Reads the validation study feature data, fits a logistic regression +on [normalized_score, author_merge_rate, log_account_age_days], and +prints the coefficients for hardcoding into V2CombinedModelConfig. + +Requirements: pandas, scikit-learn (from the validation study env). +Run: ``python scripts/train_v2_weights.py`` +""" +from __future__ import annotations + +import sys +from pathlib import Path + +import numpy as np +import pandas as pd +from sklearn.linear_model import LogisticRegression +from sklearn.metrics import roc_auc_score + +FEATURES_PATH = Path("experiments/validation/data/features/features.parquet") +FEATURE_COLS = ["normalized_score", "author_merge_rate", "log_account_age_days"] + + +def main() -> None: + if not FEATURES_PATH.exists(): + print(f"Error: {FEATURES_PATH} not found.", file=sys.stderr) + print("Run from the repo root with validation data present.", file=sys.stderr) + sys.exit(1) + + df = pd.read_parquet(FEATURES_PATH) + df["y"] = (df["outcome"] == "merged").astype(int) + + # Use subset with temporally-scoped merge rate data + mask = df["author_merge_rate"].notna() + sub = df[mask].copy() + print(f"Training on {len(sub)} PRs (of {len(df)} total)") + print(f"Base merge rate: {sub.y.mean():.3f}") + + features = sub[FEATURE_COLS].values + y = sub["y"].values + + model = LogisticRegression(C=np.inf, max_iter=10000, solver="lbfgs") + model.fit(features, y) + + print("\n=== v2 Combined Model Coefficients ===") + print(f"intercept: {model.intercept_[0]:.6f}") + for name, coef in zip(FEATURE_COLS, model.coef_[0], strict=True): + print(f"{name:24s} {coef:.6f}") + + y_pred = model.predict_proba(features)[:, 1] + print(f"\nCombined AUC: {roc_auc_score(y, y_pred):.4f}") + print(f"Graph-only AUC: {roc_auc_score(y, sub['normalized_score'].values):.4f}") + + print("\n=== For V2CombinedModelConfig defaults ===") + print(f"intercept: float = {model.intercept_[0]:.4f}") + print(f"graph_score_weight: float = {model.coef_[0][0]:.4f}") + print(f"merge_rate_weight: float = {model.coef_[0][1]:.4f}") + print(f"account_age_weight: float = {model.coef_[0][2]:.4f}") + + +if __name__ == "__main__": + main() diff --git a/src/good_egg/__init__.py b/src/good_egg/__init__.py index 55bd826..262fe0e 100644 --- a/src/good_egg/__init__.py +++ b/src/good_egg/__init__.py @@ -4,7 +4,7 @@ from importlib.metadata import PackageNotFoundError, version -from good_egg.config import GoodEggConfig +from good_egg.config import GoodEggConfig, V2Config from good_egg.exceptions import GoodEggError from good_egg.models import TrustLevel, TrustScore from good_egg.scorer import TrustScorer, score_pr_author @@ -20,6 +20,7 @@ "TrustLevel", "TrustScore", "TrustScorer", + "V2Config", "__version__", "score_pr_author", ] diff --git a/src/good_egg/action.py b/src/good_egg/action.py index de5ba1a..2a2727b 100644 --- a/src/good_egg/action.py +++ b/src/good_egg/action.py @@ -72,6 +72,12 @@ async def run_action() -> None: # Load config and score config = load_config(config_path) + scoring_model_input = ( + os.environ.get("INPUT_SCORING_MODEL") + or os.environ.get("INPUT_SCORING-MODEL") + ) + if scoring_model_input and scoring_model_input in ("v1", "v2"): + config = config.model_copy(update={"scoring_model": scoring_model_input}) cache = Cache(ttls=config.cache_ttl.to_seconds()) async with GitHubClient(token=token, config=config, cache=cache) as client: @@ -110,6 +116,7 @@ async def run_action() -> None: _set_output("score", f"{score.normalized_score:.2f}") _set_output("trust-level", score.trust_level.value) _set_output("user", score.user_login) + _set_output("scoring-model", score.scoring_model) # Summary pct = score.normalized_score * 100 diff --git a/src/good_egg/cli.py b/src/good_egg/cli.py index ac87718..762038b 100644 --- a/src/good_egg/cli.py +++ b/src/good_egg/cli.py @@ -26,6 +26,12 @@ def main() -> None: @click.option("--config", "config_path", default=None, help="Config file path") @click.option("--verbose", "-v", is_flag=True, help="Show detailed output") @click.option("--json", "output_json", is_flag=True, help="Output as JSON") +@click.option( + "--scoring-model", + type=click.Choice(["v1", "v2"]), + default=None, + help="Scoring model (v1 or v2)", +) def score( username: str, repo: str, @@ -33,6 +39,7 @@ def score( config_path: str | None, verbose: bool, output_json: bool, + scoring_model: str | None, ) -> None: """Score a GitHub user's trustworthiness relative to a repository.""" if not token: @@ -46,6 +53,8 @@ def score( repo_owner, repo_name = parts config = load_config(config_path) + if scoring_model is not None: + config = config.model_copy(update={"scoring_model": scoring_model}) cache = Cache(ttls=config.cache_ttl.to_seconds()) result = asyncio.run( diff --git a/src/good_egg/config.py b/src/good_egg/config.py index c8881ab..cb31816 100644 --- a/src/good_egg/config.py +++ b/src/good_egg/config.py @@ -4,7 +4,7 @@ import os from pathlib import Path -from typing import Any +from typing import Any, Literal import yaml from pydantic import BaseModel, Field @@ -110,8 +110,44 @@ class FetchConfig(BaseModel): rate_limit_safety_margin: int = 100 +class V2GraphConfig(BaseModel): + """Simplified graph parameters for v2 scoring.""" + half_life_days: int = 180 + max_age_days: int = 730 + archived_penalty: float = 0.5 + fork_penalty: float = 0.3 + + +class V2FeaturesConfig(BaseModel): + """Feature toggles for v2 combined model.""" + merge_rate: bool = True + account_age: bool = True + + +class V2CombinedModelConfig(BaseModel): + """Logistic regression weights for the v2 combined model. + + Trained on 5,129 PRs from the validation study (49 repos). + Features: normalized graph score, author merge rate, log(account_age_days+1). + """ + intercept: float = -0.8094 + graph_score_weight: float = 1.9138 + merge_rate_weight: float = -0.7783 + account_age_weight: float = 0.1493 + + +class V2Config(BaseModel): + """Configuration for the v2 (Better Egg) scoring model.""" + graph: V2GraphConfig = Field(default_factory=V2GraphConfig) + features: V2FeaturesConfig = Field(default_factory=V2FeaturesConfig) + combined_model: V2CombinedModelConfig = Field( + default_factory=V2CombinedModelConfig + ) + + class GoodEggConfig(BaseModel): """Top-level configuration composing all sub-configs.""" + scoring_model: Literal["v1", "v2"] = "v1" graph_scoring: GraphScoringConfig = Field(default_factory=GraphScoringConfig) edge_weights: EdgeWeightConfig = Field(default_factory=EdgeWeightConfig) recency: RecencyConfig = Field(default_factory=RecencyConfig) @@ -121,6 +157,7 @@ class GoodEggConfig(BaseModel): default_factory=LanguageNormalization ) fetch: FetchConfig = Field(default_factory=FetchConfig) + v2: V2Config = Field(default_factory=V2Config) def load_config(path: str | Path | None = None) -> GoodEggConfig: @@ -171,4 +208,9 @@ def load_config(path: str | Path | None = None) -> GoodEggConfig: config_data[section] = {} config_data[section][key] = type_fn(value) + # Top-level env var overrides + scoring_model = os.environ.get("GOOD_EGG_SCORING_MODEL") + if scoring_model is not None: + config_data["scoring_model"] = scoring_model + return GoodEggConfig(**config_data) diff --git a/src/good_egg/formatter.py b/src/good_egg/formatter.py index 25345e7..1dcda7f 100644 --- a/src/good_egg/formatter.py +++ b/src/good_egg/formatter.py @@ -25,19 +25,52 @@ } +def _brand_name(score: TrustScore) -> str: + """Return 'Better Egg' for v2, 'Good Egg' for v1.""" + return "Better Egg" if score.scoring_model == "v2" else "Good Egg" + + def format_markdown_comment(score: TrustScore) -> str: """Format a trust score as a GitHub PR comment in Markdown.""" emoji = _TRUST_LEVEL_EMOJI.get(score.trust_level, "\U0001f95a") pct = score.normalized_score * 100 + brand = _brand_name(score) lines: list[str] = [ COMMENT_MARKER, - f"## {emoji} Good Egg: **{score.trust_level.value}** Trust", + f"## {emoji} {brand}: **{score.trust_level.value}** Trust", "", f"**Score:** {pct:.0f}%", "", ] + # v2 component score breakdown + if score.scoring_model == "v2" and score.component_scores: + lines.append("### Score Breakdown") + lines.append("") + lines.append("| Component | Value |") + lines.append("|-----------|-------|") + if "graph_score" in score.component_scores: + gs = score.component_scores["graph_score"] * 100 + lines.append(f"| Graph Score | {gs:.0f}% |") + if "merge_rate" in score.component_scores: + mr = score.component_scores["merge_rate"] * 100 + merged = score.total_merged_prs + total = merged + ( + score.scoring_metadata.get("closed_pr_count", 0) + if isinstance(score.scoring_metadata.get("closed_pr_count"), int) + else 0 + ) + if total > 0: + lines.append(f"| Merge Rate | {mr:.0f}% ({merged}/{total} PRs) |") + else: + lines.append(f"| Merge Rate | {mr:.0f}% |") + if "log_account_age" in score.component_scores: + lines.append( + f"| Account Age | {score.account_age_days:,} days |" + ) + lines.append("") + # Top contributions table if score.top_contributions: lines.append("### Top Contributions") @@ -79,12 +112,13 @@ def format_cli_output(score: TrustScore, verbose: bool = False) -> str: """Format a trust score for terminal display with color.""" color = _TRUST_LEVEL_COLORS.get(score.trust_level, "white") pct = score.normalized_score * 100 + brand = _brand_name(score) trust_styled = click.style(score.trust_level.value, fg=color, bold=True) pct_styled = click.style(f"{pct:.0f}%", bold=True) lines: list[str] = [ - f"Good Egg: {trust_styled} ({pct_styled})", + f"{brand}: {trust_styled} ({pct_styled})", f"User: {score.user_login}", f"Context: {score.context_repo}", ] @@ -97,6 +131,20 @@ def format_cli_output(score: TrustScore, verbose: bool = False) -> str: f"Repos: {score.unique_repos_contributed}" ) + if score.scoring_model == "v2" and score.component_scores: + lines.append("") + lines.append("Component scores:") + if "graph_score" in score.component_scores: + gs = score.component_scores["graph_score"] * 100 + lines.append(f" Graph Score: {gs:.0f}%") + if "merge_rate" in score.component_scores: + mr = score.component_scores["merge_rate"] * 100 + lines.append(f" Merge Rate: {mr:.0f}%") + if "log_account_age" in score.component_scores: + lines.append( + f" Account Age: {score.account_age_days:,} days" + ) + if score.top_contributions: lines.append("") lines.append("Top contributions:") @@ -132,7 +180,8 @@ def format_check_run_summary(score: TrustScore) -> tuple[str, str]: A (title, summary) tuple for the Check Run API. """ pct = score.normalized_score * 100 - title = f"Good Egg: {score.trust_level.value} ({pct:.0f}%)" + brand = _brand_name(score) + title = f"{brand}: {score.trust_level.value} ({pct:.0f}%)" summary_lines: list[str] = [ f"**Trust Level:** {score.trust_level.value}", @@ -144,6 +193,20 @@ def format_check_run_summary(score: TrustScore) -> tuple[str, str]: f"Repos contributed to: {score.unique_repos_contributed}", ] + if score.scoring_model == "v2" and score.component_scores: + summary_lines.append("") + summary_lines.append("**Score Breakdown:**") + if "graph_score" in score.component_scores: + gs = score.component_scores["graph_score"] * 100 + summary_lines.append(f"- Graph Score: {gs:.0f}%") + if "merge_rate" in score.component_scores: + mr = score.component_scores["merge_rate"] * 100 + summary_lines.append(f"- Merge Rate: {mr:.0f}%") + if "log_account_age" in score.component_scores: + summary_lines.append( + f"- Account Age: {score.account_age_days:,} days" + ) + if score.top_contributions: summary_lines.append("") summary_lines.append("**Top Contributions:**") diff --git a/src/good_egg/github_client.py b/src/good_egg/github_client.py index 0697203..e3b2559 100644 --- a/src/good_egg/github_client.py +++ b/src/good_egg/github_client.py @@ -36,6 +36,7 @@ __typename followers { totalCount } repositories(first: 0) { totalCount } + closedPullRequests: pullRequests(states: CLOSED, first: 0) { totalCount } pullRequests(first: 100, states: MERGED, orderBy: {field: CREATED_AT, direction: DESC}, after: $cursor) { @@ -409,6 +410,10 @@ async def get_user_contribution_data( followers = user_data["followers"]["totalCount"] # type: ignore[index] public_repos = user_data["repositories"]["totalCount"] # type: ignore[index] created_at = datetime.fromisoformat(user_data["createdAt"]) # type: ignore[index] + closed_pr_count: int = ( + user_data.get("closedPullRequests", {}) # type: ignore[union-attr] + .get("totalCount", 0) + ) account_age = (datetime.now(UTC) - created_at).days is_suspected = ( @@ -521,6 +526,7 @@ async def get_user_contribution_data( profile=profile, merged_prs=prs, contributed_repos=contributed_repos, + closed_pr_count=closed_pr_count, ) # Cache the full contribution data diff --git a/src/good_egg/graph_builder.py b/src/good_egg/graph_builder.py index 29dad61..0229c05 100644 --- a/src/good_egg/graph_builder.py +++ b/src/good_egg/graph_builder.py @@ -16,8 +16,9 @@ class TrustGraphBuilder: MAX_PRS_PER_REPO = 20 - def __init__(self, config: GoodEggConfig) -> None: + def __init__(self, config: GoodEggConfig, simplified: bool = False) -> None: self.config = config + self.simplified = simplified def build_graph( self, user_data: UserContributionData, context_repo: str @@ -67,7 +68,7 @@ def build_graph( * quality * self.config.edge_weights.merged_pr ) - if is_self_contrib: + if is_self_contrib and not self.simplified: weight *= 0.3 # Forward edge: user -> repo @@ -97,9 +98,12 @@ def build_personalization_vector( get zero. """ pr_config = self.config.graph_scoring - adjusted_other_weight = self._compute_adjusted_other_weight( - pr_config, total_prs, unique_repos - ) + if self.simplified: + base_other_weight = pr_config.other_weight + else: + base_other_weight = self._compute_adjusted_other_weight( + pr_config, total_prs, unique_repos + ) context_node = f"repo:{context_repo}" personalization: dict[str, float] = {} @@ -115,7 +119,7 @@ def build_personalization_vector( ): personalization[node] = pr_config.same_language_weight else: - personalization[node] = adjusted_other_weight + personalization[node] = base_other_weight # Normalize to sum=1 total = sum(personalization.values()) @@ -130,8 +134,14 @@ def build_personalization_vector( def _recency_decay(self, days_ago: int) -> float: """Exponential decay based on half-life.""" - half_life = self.config.recency.half_life_days - if days_ago > self.config.recency.max_age_days: + if self.simplified: + v2g = self.config.v2.graph + half_life = v2g.half_life_days + max_age = v2g.max_age_days + else: + half_life = self.config.recency.half_life_days + max_age = self.config.recency.max_age_days + if days_ago > max_age: return 0.0 return math.exp(-0.693 * days_ago / half_life) @@ -140,13 +150,20 @@ def _repo_quality(self, meta: RepoMetadata | None) -> float: if meta is None: return 1.0 - language_mult = self._get_language_multiplier(meta.primary_language) - quality = math.log1p(meta.stargazer_count * language_mult) - - if meta.is_archived: - quality *= 0.5 - if meta.is_fork: - quality *= 0.3 + if self.simplified: + v2g = self.config.v2.graph + quality = math.log1p(meta.stargazer_count) + if meta.is_archived: + quality *= v2g.archived_penalty + if meta.is_fork: + quality *= v2g.fork_penalty + else: + language_mult = self._get_language_multiplier(meta.primary_language) + quality = math.log1p(meta.stargazer_count * language_mult) + if meta.is_archived: + quality *= 0.5 + if meta.is_fork: + quality *= 0.3 return quality diff --git a/src/good_egg/mcp_server.py b/src/good_egg/mcp_server.py index 993a2e8..7ae3989 100644 --- a/src/good_egg/mcp_server.py +++ b/src/good_egg/mcp_server.py @@ -48,6 +48,7 @@ def _error_json(message: str) -> str: @asynccontextmanager async def _scoring_resources( repo: str, + scoring_model: str | None = None, ) -> AsyncGenerator[tuple[GoodEggConfig, Cache, str, str]]: """Set up config, cache, and parsed repo for scoring tools. @@ -55,6 +56,8 @@ async def _scoring_resources( is closed on exit. """ config = _get_config() + if scoring_model is not None and scoring_model in ("v1", "v2"): + config = config.model_copy(update={"scoring_model": scoring_model}) cache = _get_cache(config) try: repo_owner, repo_name = _parse_repo(repo) @@ -77,7 +80,9 @@ async def _cache_resource() -> AsyncGenerator[Cache]: cache.close() -async def score_user(username: str, repo: str) -> str: +async def score_user( + username: str, repo: str, scoring_model: str | None = None +) -> str: """Score a GitHub user's trustworthiness relative to a repository. Returns the full trust score with all metadata as JSON. @@ -85,9 +90,10 @@ async def score_user(username: str, repo: str) -> str: Args: username: GitHub username to score. repo: Target repository in owner/repo format. + scoring_model: Optional scoring model override (v1 or v2). """ try: - async with _scoring_resources(repo) as ( + async with _scoring_resources(repo, scoring_model) as ( config, cache, repo_owner, repo_name, ): result = await score_pr_author( @@ -102,7 +108,9 @@ async def score_user(username: str, repo: str) -> str: return _error_json(str(exc)) -async def check_pr_author(username: str, repo: str) -> str: +async def check_pr_author( + username: str, repo: str, scoring_model: str | None = None +) -> str: """Quick check of a PR author's trust level. Returns a compact summary with trust level, score, and PR count. @@ -110,9 +118,10 @@ async def check_pr_author(username: str, repo: str) -> str: Args: username: GitHub username to check. repo: Target repository in owner/repo format. + scoring_model: Optional scoring model override (v1 or v2). """ try: - async with _scoring_resources(repo) as ( + async with _scoring_resources(repo, scoring_model) as ( config, cache, repo_owner, repo_name, ): result = await score_pr_author( @@ -127,13 +136,18 @@ async def check_pr_author(username: str, repo: str) -> str: "trust_level": result.trust_level.value, "normalized_score": result.normalized_score, "total_merged_prs": result.total_merged_prs, + "scoring_model": result.scoring_model, } + if result.component_scores: + summary["component_scores"] = result.component_scores return json.dumps(summary) except Exception as exc: return _error_json(str(exc)) -async def get_trust_details(username: str, repo: str) -> str: +async def get_trust_details( + username: str, repo: str, scoring_model: str | None = None +) -> str: """Get an expanded trust breakdown for a GitHub user. Returns detailed information including contributions, flags, and metadata. @@ -141,9 +155,10 @@ async def get_trust_details(username: str, repo: str) -> str: Args: username: GitHub username to analyse. repo: Target repository in owner/repo format. + scoring_model: Optional scoring model override (v1 or v2). """ try: - async with _scoring_resources(repo) as ( + async with _scoring_resources(repo, scoring_model) as ( config, cache, repo_owner, repo_name, ): result = await score_pr_author( @@ -168,6 +183,8 @@ async def get_trust_details(username: str, repo: str) -> str: ], "flags": result.flags, "scoring_metadata": result.scoring_metadata, + "scoring_model": result.scoring_model, + "component_scores": result.component_scores, } return json.dumps(details) except Exception as exc: diff --git a/src/good_egg/models.py b/src/good_egg/models.py index 98c2a76..2ab4715 100644 --- a/src/good_egg/models.py +++ b/src/good_egg/models.py @@ -65,6 +65,7 @@ class UserContributionData(BaseModel): profile: UserProfile merged_prs: list[MergedPR] = [] contributed_repos: dict[str, RepoMetadata] = {} + closed_pr_count: int = 0 class ContributionSummary(BaseModel): @@ -90,3 +91,5 @@ class TrustScore(BaseModel): language_match: bool = False flags: dict[str, bool] = {} scoring_metadata: dict[str, Any] = {} + scoring_model: str = "v1" + component_scores: dict[str, float] = {} diff --git a/src/good_egg/scorer.py b/src/good_egg/scorer.py index b80c99c..162af21 100644 --- a/src/good_egg/scorer.py +++ b/src/good_egg/scorer.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math import os from collections import defaultdict @@ -22,7 +23,6 @@ class TrustScorer: def __init__(self, config: GoodEggConfig) -> None: self.config = config - self._graph_builder = TrustGraphBuilder(config) def score( self, user_data: UserContributionData, context_repo: str @@ -38,6 +38,7 @@ def score( "has_insufficient_data": False, "used_cached_data": False, } + model_name = self.config.scoring_model # ---- Bot short-circuit ---- if user_data.profile.is_bot: @@ -49,6 +50,7 @@ def score( trust_level=TrustLevel.BOT, account_age_days=user_data.profile.account_age_days, flags=flags, + scoring_model=model_name, ) # ---- Insufficient data short-circuit ---- @@ -62,31 +64,99 @@ def score( trust_level=TrustLevel.UNKNOWN, account_age_days=user_data.profile.account_age_days, flags=flags, + scoring_model=model_name, ) # ---- Suspected bot flag ---- if user_data.profile.is_suspected_bot: flags["is_suspected_bot"] = True - # ---- Contribution stats ---- + if model_name == "v2": + return self._score_v2(user_data, context_repo, flags) + return self._score_v1(user_data, context_repo, flags) + + # ------------------------------------------------------------------ + # v1 scoring path + # ------------------------------------------------------------------ + + def _score_v1( + self, + user_data: UserContributionData, + context_repo: str, + flags: dict[str, bool], + ) -> TrustScore: + """Original graph-based scoring pipeline.""" + login = user_data.profile.login + graph_builder = TrustGraphBuilder(self.config) total_prs = len(user_data.merged_prs) unique_repos = len({pr.repo_name_with_owner for pr in user_data.merged_prs}) - # ---- Build graph ---- - graph = self._graph_builder.build_graph(user_data, context_repo) + graph = graph_builder.build_graph(user_data, context_repo) + context_language = self._resolve_context_language(user_data, context_repo) + personalization = graph_builder.build_personalization_vector( + graph, context_repo, context_language, + total_prs=total_prs, unique_repos=unique_repos, + ) - # ---- Determine context language ---- - context_language = self._resolve_context_language( - user_data, context_repo + pr_scores = nx.pagerank( + graph, + alpha=self.config.graph_scoring.alpha, + personalization=personalization if personalization else None, + weight="weight", + max_iter=self.config.graph_scoring.max_iterations, + tol=self.config.graph_scoring.tolerance, ) - # ---- Build personalization vector ---- - personalization = self._graph_builder.build_personalization_vector( + user_node = f"user:{login}" + raw_score = pr_scores.get(user_node, 0.0) + normalized = self._normalize(raw_score, graph) + trust_level = self._classify(normalized, flags) + top_contributions = self._build_top_contributions(user_data) + language_match = self._check_language_match(user_data, context_language) + + return TrustScore( + user_login=login, + context_repo=context_repo, + raw_score=raw_score, + normalized_score=normalized, + trust_level=trust_level, + account_age_days=user_data.profile.account_age_days, + total_merged_prs=total_prs, + unique_repos_contributed=unique_repos, + top_contributions=top_contributions, + language_match=language_match, + flags=flags, + scoring_metadata={ + "graph_nodes": graph.number_of_nodes(), + "graph_edges": graph.number_of_edges(), + }, + ) + + # ------------------------------------------------------------------ + # v2 scoring path (Better Egg) + # ------------------------------------------------------------------ + + def _score_v2( + self, + user_data: UserContributionData, + context_repo: str, + flags: dict[str, bool], + ) -> TrustScore: + """v2 scoring: simplified graph + logistic regression combined model.""" + login = user_data.profile.login + graph_builder = TrustGraphBuilder(self.config, simplified=True) + total_prs = len(user_data.merged_prs) + unique_repos = len({pr.repo_name_with_owner for pr in user_data.merged_prs}) + + # Step 1-2: build simplified graph + graph = graph_builder.build_graph(user_data, context_repo) + context_language = self._resolve_context_language(user_data, context_repo) + personalization = graph_builder.build_personalization_vector( graph, context_repo, context_language, total_prs=total_prs, unique_repos=unique_repos, ) - # ---- Run graph scoring ---- + # Step 3: run graph scoring pr_scores = nx.pagerank( graph, alpha=self.config.graph_scoring.alpha, @@ -95,23 +165,43 @@ def score( max_iter=self.config.graph_scoring.max_iterations, tol=self.config.graph_scoring.tolerance, ) - user_node = f"user:{login}" raw_score = pr_scores.get(user_node, 0.0) + graph_score = self._normalize(raw_score, graph) + + # Step 4: compute external features + merged_count = len(user_data.merged_prs) + closed_count = user_data.closed_pr_count + total_author_prs = merged_count + closed_count + merge_rate: float | None = ( + merged_count / total_author_prs if total_author_prs > 0 else None + ) + log_account_age = math.log(user_data.profile.account_age_days + 1) - # ---- Normalize ---- - normalized = self._normalize(raw_score, graph) + # Step 5: combined model + v2_cfg = self.config.v2 + cm = v2_cfg.combined_model + logit = cm.intercept + cm.graph_score_weight * graph_score - # ---- Classify ---- - trust_level = self._classify(normalized, flags) + if v2_cfg.features.merge_rate and merge_rate is not None: + logit += cm.merge_rate_weight * merge_rate + if v2_cfg.features.account_age: + logit += cm.account_age_weight * log_account_age + + # Sigmoid: P(merge) + normalized = 1.0 / (1.0 + math.exp(-logit)) - # ---- Build top contributions ---- + # Step 6-7: classify and build result + trust_level = self._classify(normalized, flags) top_contributions = self._build_top_contributions(user_data) + language_match = self._check_language_match(user_data, context_language) - # ---- Language match ---- - language_match = self._check_language_match( - user_data, context_language - ) + component_scores: dict[str, float] = { + "graph_score": graph_score, + } + if merge_rate is not None: + component_scores["merge_rate"] = merge_rate + component_scores["log_account_age"] = log_account_age return TrustScore( user_login=login, @@ -120,7 +210,7 @@ def score( normalized_score=normalized, trust_level=trust_level, account_age_days=user_data.profile.account_age_days, - total_merged_prs=len(user_data.merged_prs), + total_merged_prs=total_prs, unique_repos_contributed=unique_repos, top_contributions=top_contributions, language_match=language_match, @@ -128,7 +218,10 @@ def score( scoring_metadata={ "graph_nodes": graph.number_of_nodes(), "graph_edges": graph.number_of_edges(), + "closed_pr_count": closed_count, }, + scoring_model="v2", + component_scores=component_scores, ) # ------------------------------------------------------------------ diff --git a/tests/conftest.py b/tests/conftest.py index 5818ad6..7b408da 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -176,3 +176,54 @@ def sample_trust_score() -> TrustScore: }, scoring_metadata={"graph_nodes": 7, "graph_edges": 6}, ) + + +@pytest.fixture +def sample_v2_trust_score() -> TrustScore: + return TrustScore( + user_login="testuser", + context_repo="my-org/my-elixir-app", + raw_score=0.0045, + normalized_score=0.72, + trust_level=TrustLevel.HIGH, + percentile=85.0, + account_age_days=1800, + total_merged_prs=3, + unique_repos_contributed=3, + top_contributions=[ + ContributionSummary( + repo_name="elixir-lang/elixir", + pr_count=1, + language="Elixir", + stars=23000, + ), + ], + language_match=True, + flags={ + "is_bot": False, + "is_new_account": False, + "has_insufficient_data": False, + "used_cached_data": False, + }, + scoring_metadata={"graph_nodes": 7, "graph_edges": 6}, + scoring_model="v2", + component_scores={ + "graph_score": 0.65, + "merge_rate": 0.85, + "log_account_age": 7.5, + }, + ) + + +@pytest.fixture +def sample_v2_contribution_data( + sample_user_profile: UserProfile, + sample_merged_prs: list[MergedPR], + sample_repos_metadata: dict[str, RepoMetadata], +) -> UserContributionData: + return UserContributionData( + profile=sample_user_profile, + merged_prs=sample_merged_prs, + contributed_repos=sample_repos_metadata, + closed_pr_count=5, + ) diff --git a/tests/fixtures/user_prs.json b/tests/fixtures/user_prs.json index bc6d511..c3d49f9 100644 --- a/tests/fixtures/user_prs.json +++ b/tests/fixtures/user_prs.json @@ -6,6 +6,7 @@ "__typename": "User", "followers": {"totalCount": 50}, "repositories": {"totalCount": 20}, + "closedPullRequests": {"totalCount": 5}, "pullRequests": { "totalCount": 3, "pageInfo": {"hasNextPage": false, "endCursor": null}, diff --git a/tests/test_action.py b/tests/test_action.py index 62a0596..44549f7 100644 --- a/tests/test_action.py +++ b/tests/test_action.py @@ -147,6 +147,36 @@ async def test_sets_outputs(self, mock_env, tmp_path): assert "trust-level=HIGH" in output_content assert "user=testuser" in output_content + @pytest.mark.asyncio + async def test_scoring_model_input(self, mock_env, tmp_path): + """INPUT_SCORING_MODEL should be read and applied.""" + output_file = tmp_path / "output.txt" + output_file.touch() + mock_env_v2 = { + **mock_env, + "INPUT_SCORING_MODEL": "v2", + "GITHUB_OUTPUT": str(output_file), + } + mock_score = _make_mock_score() + mock_score = TrustScore( + **{**mock_score.model_dump(), "scoring_model": "v2"} + ) + mock_client = AsyncMock() + mock_client.get_user_contribution_data = AsyncMock() + mock_client.find_existing_comment = AsyncMock(return_value=None) + mock_client.post_pr_comment = AsyncMock(return_value={}) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock(return_value=False) + + with patch.dict(os.environ, mock_env_v2, clear=False), \ + patch("good_egg.action.GitHubClient", return_value=mock_client), \ + patch("good_egg.action.TrustScorer") as mock_scorer_cls: + mock_scorer_cls.return_value.score.return_value = mock_score + await run_action() + + output_content = output_file.read_text() + assert "scoring-model=v2" in output_content + class TestSetOutput: def test_writes_to_file(self, tmp_path): diff --git a/tests/test_cli.py b/tests/test_cli.py index 5a561cb..178aefe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -100,6 +100,24 @@ def test_score_command_verbose( assert "365" in result.output assert "Merged PRs" in result.output + @patch("good_egg.cli.score_pr_author", new_callable=AsyncMock) + @patch("good_egg.cli.load_config") + def test_scoring_model_v2_option( + self, mock_load_config: MagicMock, mock_score: AsyncMock + ) -> None: + """The --scoring-model v2 flag should be accepted.""" + mock_config = MagicMock() + mock_load_config.return_value = mock_config + trust_score = _make_trust_score() + mock_score.return_value = trust_score + + runner = CliRunner(env={"GITHUB_TOKEN": "ghp_fake123"}) + result = runner.invoke( + main, + ["score", "testuser", "--repo", "owner/repo", "--scoring-model", "v2"], + ) + assert result.exit_code == 0 + class TestCacheCommands: @patch("good_egg.cli.Cache") diff --git a/tests/test_config.py b/tests/test_config.py index 85f4af7..fcdb160 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -128,3 +128,69 @@ def test_env_var_volume_scale(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("GOOD_EGG_VOLUME_SCALE", "0.5") config = load_config() assert config.graph_scoring.volume_scale == 0.5 + + +class TestV2Config: + def test_defaults(self) -> None: + from good_egg.config import V2Config + config = V2Config() + assert config.graph.half_life_days == 180 + assert config.graph.max_age_days == 730 + assert config.graph.archived_penalty == 0.5 + assert config.graph.fork_penalty == 0.3 + assert config.features.merge_rate is True + assert config.features.account_age is True + assert isinstance(config.combined_model.intercept, float) + assert isinstance(config.combined_model.graph_score_weight, float) + + def test_custom_values(self) -> None: + from good_egg.config import V2Config + config = V2Config( + graph={"half_life_days": 90, "archived_penalty": 0.2}, + features={"merge_rate": False}, + ) + assert config.graph.half_life_days == 90 + assert config.graph.archived_penalty == 0.2 + assert config.features.merge_rate is False + # Defaults preserved + assert config.graph.max_age_days == 730 + + +class TestScoringModelConfig: + def test_default_is_v1(self) -> None: + config = GoodEggConfig() + assert config.scoring_model == "v1" + + def test_set_to_v2(self) -> None: + config = GoodEggConfig(scoring_model="v2") + assert config.scoring_model == "v2" + + def test_v2_config_present_by_default(self) -> None: + config = GoodEggConfig() + assert config.v2 is not None + assert config.v2.graph.half_life_days == 180 + + def test_yaml_loading_with_v2(self, tmp_path: Path) -> None: + config_file = tmp_path / ".good-egg.yml" + config_file.write_text(yaml.dump({ + "scoring_model": "v2", + "v2": { + "graph": {"half_life_days": 90}, + "features": {"account_age": False}, + }, + })) + config = load_config(config_file) + assert config.scoring_model == "v2" + assert config.v2.graph.half_life_days == 90 + assert config.v2.features.account_age is False + + def test_env_var_scoring_model(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GOOD_EGG_SCORING_MODEL", "v2") + config = load_config() + assert config.scoring_model == "v2" + + def test_v2_config_ignored_when_v1(self) -> None: + config = GoodEggConfig(scoring_model="v1") + # v2 config is still present but not used by v1 scorer + assert config.v2 is not None + assert config.scoring_model == "v1" diff --git a/tests/test_formatter.py b/tests/test_formatter.py index 26d0405..88a5c59 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -189,3 +189,96 @@ def test_no_flags_section_when_empty(self) -> None: score = _make_score(flags={}) _, summary = format_check_run_summary(score) assert "Flags" not in summary + + +class TestBetterEggFormatting: + def _make_v2_score(self, **kwargs) -> TrustScore: + defaults = { + "user_login": "testuser", + "context_repo": "owner/repo", + "raw_score": 0.65, + "normalized_score": 0.72, + "trust_level": TrustLevel.HIGH, + "percentile": 85.0, + "account_age_days": 1825, + "total_merged_prs": 42, + "unique_repos_contributed": 10, + "top_contributions": [ + ContributionSummary( + repo_name="cool/project", + pr_count=15, + language="Python", + stars=1200, + ), + ], + "language_match": True, + "flags": {}, + "scoring_metadata": {"closed_pr_count": 8}, + "scoring_model": "v2", + "component_scores": { + "graph_score": 0.65, + "merge_rate": 0.85, + "log_account_age": 7.5, + }, + } + defaults.update(kwargs) + return TrustScore(**defaults) + + def test_markdown_better_egg_header(self) -> None: + score = self._make_v2_score() + md = format_markdown_comment(score) + assert "Better Egg" in md + assert "Good Egg" not in md.split("Better Egg")[0] # No "Good Egg" before Better Egg + + def test_markdown_component_breakdown(self) -> None: + score = self._make_v2_score() + md = format_markdown_comment(score) + assert "Score Breakdown" in md + assert "Graph Score" in md + assert "Merge Rate" in md + assert "Account Age" in md + assert "65%" in md # graph_score * 100 + assert "85% (42/50 PRs)" in md # merge_rate with closed_pr_count=8 + + def test_cli_better_egg_header(self) -> None: + score = self._make_v2_score() + out = format_cli_output(score) + assert "Better Egg" in out + + def test_cli_verbose_component_scores(self) -> None: + score = self._make_v2_score() + out = format_cli_output(score, verbose=True) + assert "Component scores" in out + assert "Graph Score" in out + assert "Merge Rate" in out + + def test_check_run_better_egg_title(self) -> None: + score = self._make_v2_score() + title, summary = format_check_run_summary(score) + assert "Better Egg" in title + assert "Score Breakdown" in summary + + def test_json_includes_v2_fields(self) -> None: + score = self._make_v2_score() + result = format_json(score) + parsed = json.loads(result) + assert parsed["scoring_model"] == "v2" + assert "graph_score" in parsed["component_scores"] + + def test_v1_format_unchanged(self) -> None: + score = _make_score() + md = format_markdown_comment(score) + assert "Good Egg" in md + assert "Better Egg" not in md + assert "Score Breakdown" not in md + + def test_v1_cli_unchanged(self) -> None: + score = _make_score() + out = format_cli_output(score) + assert "Good Egg" in out + assert "Better Egg" not in out + + def test_v1_check_run_unchanged(self) -> None: + score = _make_score() + title, _ = format_check_run_summary(score) + assert title == "Good Egg: HIGH (72%)" diff --git a/tests/test_github_client.py b/tests/test_github_client.py index 3ecd120..a4bb2ec 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -406,6 +406,51 @@ async def test_null_repository_skipped(self) -> None: assert data.merged_prs[0].title == "PR to accessible repo" assert "elixir-lang/elixir" in data.contributed_repos + @respx.mock + async def test_closed_pr_count_parsed(self) -> None: + """closedPullRequests totalCount should be parsed into closed_pr_count.""" + fixture = { + "data": { + "user": { + "login": "testuser", + "createdAt": "2020-01-01T00:00:00Z", + "__typename": "User", + "followers": {"totalCount": 50}, + "repositories": {"totalCount": 20}, + "closedPullRequests": {"totalCount": 12}, + "pullRequests": { + "totalCount": 3, + "pageInfo": {"hasNextPage": False, "endCursor": None}, + "nodes": [ + { + "title": "Fix bug", + "mergedAt": "2024-06-15T00:00:00Z", + "additions": 10, + "deletions": 5, + "changedFiles": 1, + "repository": { + "nameWithOwner": "elixir-lang/elixir", + "stargazerCount": 23000, + "forkCount": 3200, + "primaryLanguage": {"name": "Elixir"}, + "isArchived": False, + "isFork": False, + }, + }, + ], + }, + } + } + } + respx.post(GRAPHQL_URL).mock( + return_value=httpx.Response(200, json=fixture) + ) + + async with _make_client() as client: + data = await client.get_user_contribution_data("testuser") + + assert data.closed_pr_count == 12 + # --------------------------------------------------------------------------- # REST: PR comments diff --git a/tests/test_graph_builder.py b/tests/test_graph_builder.py index f57ae6e..eeb21f4 100644 --- a/tests/test_graph_builder.py +++ b/tests/test_graph_builder.py @@ -443,3 +443,188 @@ def test_prolific_gets_higher_personalization_share(self) -> None: ) # The Python repo is "other" (not Rust), so prolific should get more weight on it assert pv_prolific["repo:org/python-lib"] > pv_newcomer["repo:org/python-lib"] + + +class TestSimplifiedMode: + def test_no_self_contribution_penalty(self) -> None: + builder = TrustGraphBuilder(_make_config(), simplified=True) + pr_own = MergedPR( + repo_name_with_owner="testuser/my-project", + title="Update readme", + merged_at=datetime.now(UTC) - timedelta(days=5), + ) + pr_external = MergedPR( + repo_name_with_owner="other-org/project", + title="Fix bug", + merged_at=datetime.now(UTC) - timedelta(days=5), + ) + repos = { + "testuser/my-project": RepoMetadata( + name_with_owner="testuser/my-project", + stargazer_count=100, + primary_language="Python", + ), + "other-org/project": RepoMetadata( + name_with_owner="other-org/project", + stargazer_count=100, + primary_language="Python", + ), + } + data = _make_user_data( + login="testuser", + merged_prs=[pr_own, pr_external], + repos=repos, + ) + graph = builder.build_graph(data, "ctx/repo") + + own_weight = graph["user:testuser"]["repo:testuser/my-project"]["weight"] + ext_weight = graph["user:testuser"]["repo:other-org/project"]["weight"] + # In simplified mode, no 0.3 penalty on self-contributions + assert abs(own_weight - ext_weight) < 1e-9 + + def test_no_language_normalization_in_repo_quality(self) -> None: + config = _make_config() + builder_simple = TrustGraphBuilder(config, simplified=True) + builder_v1 = TrustGraphBuilder(config, simplified=False) + + meta = RepoMetadata( + name_with_owner="org/repo", + stargazer_count=10000, + primary_language="Elixir", # Has a 4.04 multiplier in v1 + ) + + q_simple = builder_simple._repo_quality(meta) + q_v1 = builder_v1._repo_quality(meta) + + # Simplified uses log1p(10000) without language multiplier + assert abs(q_simple - math.log1p(10000)) < 1e-9 + # v1 uses log1p(10000 * 4.04) + assert abs(q_v1 - math.log1p(10000 * 4.04)) < 1e-9 + assert q_v1 > q_simple + + def test_same_language_weight_retained_in_personalization(self) -> None: + builder = TrustGraphBuilder(_make_config(), simplified=True) + pr1 = MergedPR( + repo_name_with_owner="org/elixir-lib", + title="PR", + merged_at=datetime.now(UTC), + ) + pr2 = MergedPR( + repo_name_with_owner="org/python-lib", + title="PR", + merged_at=datetime.now(UTC), + ) + repos = { + "org/elixir-lib": RepoMetadata( + name_with_owner="org/elixir-lib", + stargazer_count=100, + primary_language="Elixir", + ), + "org/python-lib": RepoMetadata( + name_with_owner="org/python-lib", + stargazer_count=100, + primary_language="Python", + ), + } + data = _make_user_data(merged_prs=[pr1, pr2], repos=repos) + graph = builder.build_graph(data, "ctx/repo") + + pv = builder.build_personalization_vector(graph, "ctx/repo", "Elixir") + + # In simplified mode, same_language_weight is now retained, + # so the Elixir repo should get higher weight than the Python repo + elixir_weight = pv["repo:org/elixir-lib"] + python_weight = pv["repo:org/python-lib"] + assert elixir_weight > python_weight + + def test_no_diversity_volume_adjustment(self) -> None: + builder = TrustGraphBuilder(_make_config(), simplified=True) + pr = MergedPR( + repo_name_with_owner="org/repo", + title="PR", + merged_at=datetime.now(UTC), + ) + repos = { + "org/repo": RepoMetadata( + name_with_owner="org/repo", + stargazer_count=100, + primary_language="Python", + ), + } + data = _make_user_data(merged_prs=[pr], repos=repos) + graph = builder.build_graph(data, "ctx/repo") + + pv_few = builder.build_personalization_vector( + graph, "ctx/repo", "Rust", total_prs=2, unique_repos=1 + ) + pv_many = builder.build_personalization_vector( + graph, "ctx/repo", "Rust", total_prs=100, unique_repos=20 + ) + # In simplified mode, total_prs/unique_repos don't affect other_weight + # Both get the same weight for the non-context repo + assert abs(pv_few["repo:org/repo"] - pv_many["repo:org/repo"]) < 1e-9 + + def test_v2_graph_config_penalties(self) -> None: + from good_egg.config import GoodEggConfig + config = GoodEggConfig( + v2={"graph": {"archived_penalty": 0.2, "fork_penalty": 0.1}} + ) + builder = TrustGraphBuilder(config, simplified=True) + + meta_archived = RepoMetadata( + name_with_owner="org/repo", + stargazer_count=1000, + is_archived=True, + ) + meta_fork = RepoMetadata( + name_with_owner="org/repo", + stargazer_count=1000, + is_fork=True, + ) + meta_normal = RepoMetadata( + name_with_owner="org/repo", + stargazer_count=1000, + ) + + q_normal = builder._repo_quality(meta_normal) + q_archived = builder._repo_quality(meta_archived) + q_fork = builder._repo_quality(meta_fork) + + assert abs(q_archived - q_normal * 0.2) < 1e-9 + assert abs(q_fork - q_normal * 0.1) < 1e-9 + + def test_v1_mode_unchanged(self) -> None: + """Ensure non-simplified mode still applies self-contribution penalty.""" + builder = TrustGraphBuilder(_make_config(), simplified=False) + pr_own = MergedPR( + repo_name_with_owner="testuser/my-project", + title="Update readme", + merged_at=datetime.now(UTC) - timedelta(days=5), + ) + pr_external = MergedPR( + repo_name_with_owner="other-org/project", + title="Fix bug", + merged_at=datetime.now(UTC) - timedelta(days=5), + ) + repos = { + "testuser/my-project": RepoMetadata( + name_with_owner="testuser/my-project", + stargazer_count=100, + primary_language="Python", + ), + "other-org/project": RepoMetadata( + name_with_owner="other-org/project", + stargazer_count=100, + primary_language="Python", + ), + } + data = _make_user_data( + login="testuser", + merged_prs=[pr_own, pr_external], + repos=repos, + ) + graph = builder.build_graph(data, "ctx/repo") + + own_weight = graph["user:testuser"]["repo:testuser/my-project"]["weight"] + ext_weight = graph["user:testuser"]["repo:other-org/project"]["weight"] + assert abs(own_weight - ext_weight * 0.3) < 1e-9 diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 29be4c7..66dd1d8 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -167,6 +167,27 @@ async def test_scoring_error( assert "API failure" in parsed["error"] mock_cache_inst.close.assert_called_once() + @pytest.mark.asyncio + @patch("good_egg.mcp_server._get_cache") + @patch("good_egg.mcp_server._get_config") + @patch("good_egg.mcp_server.score_pr_author", new_callable=AsyncMock) + async def test_scoring_model_parameter( + self, + mock_score: AsyncMock, + mock_config: MagicMock, + mock_cache: MagicMock, + ) -> None: + mock_config.return_value = MagicMock() + mock_cache_inst = MagicMock() + mock_cache.return_value = mock_cache_inst + trust = _make_trust_score() + mock_score.return_value = trust + + result = await score_user("testuser", "owner/repo", scoring_model="v2") + parsed = json.loads(result) + assert parsed["user_login"] == "testuser" + mock_cache_inst.close.assert_called_once() + class TestCheckPrAuthor: @pytest.mark.asyncio @@ -221,6 +242,36 @@ async def test_scoring_error( assert "error" in parsed mock_cache_inst.close.assert_called_once() + @pytest.mark.asyncio + @patch("good_egg.mcp_server._get_cache") + @patch("good_egg.mcp_server._get_config") + @patch("good_egg.mcp_server.score_pr_author", new_callable=AsyncMock) + async def test_v2_fields_in_response( + self, + mock_score: AsyncMock, + mock_config: MagicMock, + mock_cache: MagicMock, + ) -> None: + mock_config.return_value = MagicMock() + mock_cache_inst = MagicMock() + mock_cache.return_value = mock_cache_inst + trust = TrustScore( + user_login="testuser", + context_repo="owner/repo", + normalized_score=0.72, + trust_level=TrustLevel.HIGH, + total_merged_prs=42, + scoring_model="v2", + component_scores={"graph_score": 0.65}, + ) + mock_score.return_value = trust + + result = await check_pr_author("testuser", "owner/repo", scoring_model="v2") + parsed = json.loads(result) + assert parsed["scoring_model"] == "v2" + assert parsed["component_scores"]["graph_score"] == 0.65 + mock_cache_inst.close.assert_called_once() + class TestGetTrustDetails: @pytest.mark.asyncio @@ -281,6 +332,42 @@ async def test_scoring_error( assert "error" in parsed mock_cache_inst.close.assert_called_once() + @pytest.mark.asyncio + @patch("good_egg.mcp_server._get_cache") + @patch("good_egg.mcp_server._get_config") + @patch("good_egg.mcp_server.score_pr_author", new_callable=AsyncMock) + async def test_v2_fields_in_response( + self, + mock_score: AsyncMock, + mock_config: MagicMock, + mock_cache: MagicMock, + ) -> None: + mock_config.return_value = MagicMock() + mock_cache_inst = MagicMock() + mock_cache.return_value = mock_cache_inst + trust = TrustScore( + user_login="testuser", + context_repo="owner/repo", + raw_score=0.005, + normalized_score=0.72, + trust_level=TrustLevel.HIGH, + account_age_days=365, + total_merged_prs=42, + unique_repos_contributed=10, + language_match=True, + flags={"is_bot": False}, + scoring_metadata={"graph_nodes": 50}, + scoring_model="v2", + component_scores={"graph_score": 0.65, "merge_rate": 0.8}, + ) + mock_score.return_value = trust + + result = await get_trust_details("testuser", "owner/repo", scoring_model="v2") + parsed = json.loads(result) + assert parsed["scoring_model"] == "v2" + assert parsed["component_scores"]["graph_score"] == 0.65 + mock_cache_inst.close.assert_called_once() + class TestCacheStats: @pytest.mark.asyncio diff --git a/tests/test_models.py b/tests/test_models.py index 36c65a2..4dac06c 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -89,6 +89,16 @@ def test_empty_defaults(self, sample_user_profile: UserProfile) -> None: assert data.merged_prs == [] assert data.contributed_repos == {} + def test_closed_pr_count_default(self, sample_user_profile: UserProfile) -> None: + data = UserContributionData(profile=sample_user_profile) + assert data.closed_pr_count == 0 + + def test_closed_pr_count_set(self, sample_user_profile: UserProfile) -> None: + data = UserContributionData( + profile=sample_user_profile, closed_pr_count=15 + ) + assert data.closed_pr_count == 15 + class TestTrustScore: def test_creation(self, sample_trust_score: TrustScore) -> None: @@ -106,6 +116,46 @@ def test_defaults(self) -> None: assert score.trust_level == TrustLevel.UNKNOWN assert score.flags == {} + def test_scoring_model_default(self) -> None: + score = TrustScore(user_login="u", context_repo="o/r") + assert score.scoring_model == "v1" + assert score.component_scores == {} + + def test_scoring_model_v2(self) -> None: + score = TrustScore( + user_login="u", + context_repo="o/r", + scoring_model="v2", + component_scores={"graph_score": 0.65, "merge_rate": 0.8}, + ) + assert score.scoring_model == "v2" + assert score.component_scores["graph_score"] == 0.65 + + def test_serialization_roundtrip_v2(self) -> None: + import json + score = TrustScore( + user_login="u", + context_repo="o/r", + scoring_model="v2", + component_scores={"graph_score": 0.65}, + ) + data = json.loads(score.model_dump_json()) + restored = TrustScore(**data) + assert restored.scoring_model == "v2" + assert restored.component_scores == {"graph_score": 0.65} + + def test_backward_compat_no_new_fields(self) -> None: + # Old-style TrustScore without new fields still works + score = TrustScore( + user_login="u", + context_repo="o/r", + raw_score=0.5, + normalized_score=0.7, + trust_level=TrustLevel.HIGH, + ) + assert score.scoring_model == "v1" + assert score.component_scores == {} + def test_top_contributions(self, sample_trust_score: TrustScore) -> None: assert len(sample_trust_score.top_contributions) == 2 assert sample_trust_score.top_contributions[0].repo_name == "elixir-lang/elixir" diff --git a/tests/test_scorer.py b/tests/test_scorer.py index 25d283c..9bb33fb 100644 --- a/tests/test_scorer.py +++ b/tests/test_scorer.py @@ -39,11 +39,13 @@ def _make_contribution_data( days_old: int = 500, merged_prs: list[MergedPR] | None = None, repos: dict[str, RepoMetadata] | None = None, + closed_pr_count: int = 0, ) -> UserContributionData: return UserContributionData( profile=_make_profile(login=login, is_bot=is_bot, days_old=days_old), merged_prs=merged_prs or [], contributed_repos=repos or {}, + closed_pr_count=closed_pr_count, ) @@ -414,3 +416,227 @@ def test_diverse_cross_ecosystem_user_gets_nonzero_score(self) -> None: assert result.normalized_score > 0.0 assert result.total_merged_prs == 20 assert result.unique_repos_contributed == 20 + + +class TestV2Scoring: + def test_v2_scoring_produces_nonzero_score(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=5) + result = scorer.score(data, "my-org/my-elixir-app") + + assert result.raw_score > 0.0 + assert 0.0 <= result.normalized_score <= 1.0 + assert result.scoring_model == "v2" + + def test_v2_component_scores_populated(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=5) + result = scorer.score(data, "my-org/my-elixir-app") + + assert "graph_score" in result.component_scores + assert "merge_rate" in result.component_scores + assert "log_account_age" in result.component_scores + assert 0.0 <= result.component_scores["graph_score"] <= 1.0 + + def test_v2_merge_rate_calculation(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + # 3 merged PRs + 2 closed = 5 total, merge rate = 3/5 = 0.6 + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=2) + result = scorer.score(data, "my-org/my-elixir-app") + + assert abs(result.component_scores["merge_rate"] - 0.6) < 1e-9 + + def test_v2_merge_rate_zero_denominator(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + prs = [ + MergedPR( + repo_name_with_owner="org/repo", + title="PR", + merged_at=datetime.now(UTC) - timedelta(days=1), + ), + ] + repos = { + "org/repo": RepoMetadata( + name_with_owner="org/repo", + stargazer_count=100, + ), + } + # 1 merged + 0 closed = should use merged_count / (merged + closed) + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=0) + result = scorer.score(data, "org/repo") + + # merge_rate = 1 / (1 + 0) = 1.0 + assert abs(result.component_scores["merge_rate"] - 1.0) < 1e-9 + + def test_v2_feature_disabled_merge_rate(self) -> None: + config = GoodEggConfig( + scoring_model="v2", + v2={"features": {"merge_rate": False}}, + ) + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=5) + result = scorer.score(data, "my-org/app") + + # merge_rate is still in component_scores for transparency + assert "merge_rate" in result.component_scores + assert result.scoring_model == "v2" + + def test_v2_bot_short_circuit(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + data = _make_contribution_data(is_bot=True) + result = scorer.score(data, "org/repo") + + assert result.trust_level == TrustLevel.BOT + assert result.scoring_model == "v2" + + def test_v2_unknown_short_circuit(self) -> None: + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + data = _make_contribution_data(merged_prs=[]) + result = scorer.score(data, "org/repo") + + assert result.trust_level == TrustLevel.UNKNOWN + assert result.scoring_model == "v2" + + def test_v1_mode_unchanged(self) -> None: + """Regression: v1 scoring unchanged.""" + config = GoodEggConfig(scoring_model="v1") + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos) + result = scorer.score(data, "my-org/my-elixir-app") + + assert result.scoring_model == "v1" + assert result.component_scores == {} + assert result.raw_score > 0.0 + + def test_v2_all_features_disabled(self) -> None: + """With both features disabled, logit uses only intercept + graph_score.""" + config = GoodEggConfig( + scoring_model="v2", + v2={"features": {"merge_rate": False, "account_age": False}}, + ) + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=5) + result = scorer.score(data, "my-org/my-elixir-app") + + # Should still produce a valid sigmoid score in (0, 1) + assert 0.0 < result.normalized_score < 1.0 + assert result.scoring_model == "v2" + # Component scores should still be populated for transparency + assert "graph_score" in result.component_scores + assert "merge_rate" in result.component_scores + assert "log_account_age" in result.component_scores + + def test_v2_negative_merge_rate_weight_behavior(self) -> None: + """The negative merge_rate_weight means higher merge rate lowers the logit contribution. + + User with 100% merge rate vs user with 50% merge rate: the one with + higher merge rate gets a more negative contribution from merge_rate_weight. + Both should produce valid scores in (0, 1). + """ + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + + prs = [ + MergedPR( + repo_name_with_owner="org/repo", + title="PR", + merged_at=datetime.now(UTC) - timedelta(days=10), + ) + for _ in range(10) + ] + repos = { + "org/repo": RepoMetadata( + name_with_owner="org/repo", + stargazer_count=5000, + primary_language="Python", + ), + } + + # User A: 100% merge rate (10 merged, 0 closed) + data_high_mr = _make_contribution_data( + merged_prs=prs, repos=repos, closed_pr_count=0 + ) + result_high_mr = scorer.score(data_high_mr, "org/repo") + + # User B: 50% merge rate (10 merged, 10 closed) + data_low_mr = _make_contribution_data( + merged_prs=prs, repos=repos, closed_pr_count=10 + ) + result_low_mr = scorer.score(data_low_mr, "org/repo") + + # Both produce valid scores + assert 0.0 < result_high_mr.normalized_score < 1.0 + assert 0.0 < result_low_mr.normalized_score < 1.0 + + # With negative merge_rate_weight (-0.7783), higher merge rate means + # more negative contribution, so the high-merge-rate user gets a LOWER score + assert result_high_mr.normalized_score < result_low_mr.normalized_score + + def test_v2_opposing_signals(self) -> None: + """User with high graph score but low merge rate (many closed PRs). + + The combined model should still produce a valid score in (0, 1) + and component_scores should be populated correctly. + """ + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + + # Many merged PRs to popular repos -> high graph score + prs = [ + MergedPR( + repo_name_with_owner=f"big-org/popular-{i}", + title=f"Major contribution {i}", + merged_at=datetime.now(UTC) - timedelta(days=i * 5), + ) + for i in range(15) + ] + repos = { + f"big-org/popular-{i}": RepoMetadata( + name_with_owner=f"big-org/popular-{i}", + stargazer_count=10000 + i * 1000, + primary_language="Python", + ) + for i in range(15) + } + + # 15 merged + 30 closed = 33% merge rate (low) + data = _make_contribution_data( + merged_prs=prs, repos=repos, closed_pr_count=30 + ) + result = scorer.score(data, "big-org/popular-0") + + # Valid score in (0, 1) + assert 0.0 < result.normalized_score < 1.0 + assert result.scoring_model == "v2" + + # Component scores are populated + assert "graph_score" in result.component_scores + assert "merge_rate" in result.component_scores + assert "log_account_age" in result.component_scores + + # Merge rate should be 15 / (15 + 30) = 1/3 + expected_mr = 15.0 / 45.0 + assert abs(result.component_scores["merge_rate"] - expected_mr) < 1e-9 + + def test_v2_sigmoid_computation(self) -> None: + """Verify the combined model produces reasonable sigmoid output.""" + config = GoodEggConfig(scoring_model="v2") + scorer = TrustScorer(config) + prs, repos = _sample_prs_and_repos() + data = _make_contribution_data(merged_prs=prs, repos=repos, closed_pr_count=5) + result = scorer.score(data, "my-org/my-elixir-app") + + # The normalized score should be a valid probability from sigmoid + assert 0.0 < result.normalized_score < 1.0