Skip to content

fix(signals): include recent merged PRs in outcome analysis - #313

Merged
JSONbored merged 4 commits into
JSONbored:mainfrom
philluiz2323:fix/repo-outcome-include-merged
Jun 3, 2026
Merged

fix(signals): include recent merged PRs in outcome analysis#313
JSONbored merged 4 commits into
JSONbored:mainfrom
philluiz2323:fix/repo-outcome-include-merged

Conversation

@philluiz2323

Copy link
Copy Markdown
Contributor

Closes #312.

Problem

buildRepoOutcomePatterns built its analyzed PR set from only args.pullRequests and used recentMergedPullRequests purely as a by-number enrichment lookup. But the loader reads pullRequests (the pull_requests table) and recentMergedPullRequests (the separate recent_merged_pull_requests table) independently, and the backfill writes open PRs to pull_requests (marking vanished ones closed with no mergedAt) while merged PRs go to the separate table. So a repo's historical merged PRs were absent from the analysis: outsideContributorMergeRate collapsed toward 0, falsely firing the "Outside contributor PRs rarely merge here / high closure risk" pattern and suppressing "merge well here", and every outcome dimension was blind to merged history. This feeds buildContributorStrategy, repo-fit, reward-risk, and the decision pack, so healthy repos were reported as high-closure-risk.

Fix

  • New normalizeRecentMergedOutcome helper converts a recent_merged_pull_requests record into a decided/merged RepoOutcomePullRequest, deriving maintainer-lane / author-role from the stored payload's author_association (the table has no association column).
  • The analyzed set is now the union of pull_requests rows and merged records that exist only in recent_merged_pull_requests (deduped by PR number, requiring a real mergedAt).
  • Reconciliation fix: a PR with a genuine recent-merged record (carrying mergedAt) is counted as merged, not closed_unmerged, even when the open-PR reconciliation only saw it vanish. A recent-merged enrichment record with mergedAt: null does not flip a PR to merged.

Tests

Added a fail-on-revert test: with only closed shells in pullRequests and 9 merged PRs living only in recent_merged_pull_requests, the result must report totals.merged: 9 and the "Outside contributors merge well here" success pattern, not the false high-closure-risk pattern. The existing recent-merged tests only used merged PRs that also appear in pullRequests, so they never exercised the merged-only path.

vitest run repo-outcome-patterns (+service) 30/30; consumers + e2e (signals, signals-v2, decision-pack, queue, integration/api) 143/143; tsc --noEmit clean.

@philluiz2323
philluiz2323 force-pushed the fix/repo-outcome-include-merged branch from c9dc438 to beea306 Compare June 2, 2026 15:41
@JSONbored JSONbored added the bug label Jun 2, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philluiz2323 the fix looks useful and CI is green, but the PR title needs to follow the repo title convention.

Required change:

  • Retitle this to something like fix(signals): include recent merged PRs in outcome analysis.

Validation expected:

  • Keep npm run test:ci green after any update.

@philluiz2323 philluiz2323 changed the title Include recent_merged_pull_requests in repo outcome merge-rate analysis fix(signals): include recent merged PRs in outcome analysis. Jun 2, 2026
@philluiz2323
philluiz2323 requested a review from JSONbored June 2, 2026 17:45
@philluiz2323

Copy link
Copy Markdown
Contributor Author

@philluiz2323 the fix looks useful and CI is green, but the PR title needs to follow the repo title convention.

Required change:

  • Retitle this to something like fix(signals): include recent merged PRs in outcome analysis.

Validation expected:

  • Keep npm run test:ci green after any update.

done

@JSONbored JSONbored changed the title fix(signals): include recent merged PRs in outcome analysis. fix(signals): include recent merged PRs in outcome analysis Jun 3, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philluiz2323 this still needs cleanup before it can be reviewed.

A few notes:

  • The problem area is valid.
  • The title has a trailing period and does not meet this repo's PR-title policy. (fixed)
  • The PR body is missing the current template evidence.

Required changes:

  • None for now.

Validation expected:

  • Focused repo outcome pattern tests.
  • Full CI green after any rebase.

@dosubot dosubot Bot added the lgtm label Jun 3, 2026
@JSONbored
JSONbored merged commit b0fb7fc into JSONbored:main Jun 3, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 3, 2026
@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

2 participants