Skip to content

fix(signals): include recent_merged_pull_requests in outcome pattern - #316

Merged
JSONbored merged 13 commits into
JSONbored:mainfrom
enjoyandlove:feat/82-beta-grade-base-agent-v2
Jun 4, 2026
Merged

fix(signals): include recent_merged_pull_requests in outcome pattern#316
JSONbored merged 13 commits into
JSONbored:mainfrom
enjoyandlove:feat/82-beta-grade-base-agent-v2

Conversation

@enjoyandlove

@enjoyandlove enjoyandlove commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • buildRepoOutcomePatterns iterated only args.pullRequests and used
    recentMergedPullRequests purely as an enrichment lookup for PRs already
    present. Any PR that existed only in the recent_merged_pull_requests table
    was silently dropped from every count and rate, collapsing the
    outside-contributor merge rate toward zero and falsely triggering
    "Outside contributor PRs rarely merge here" risk patterns on healthy repos.
  • Fixed by building a unified decided-PR set: PRs absent from pullRequests
    but present in recentMergedPullRequests are appended as bucket: "merged".
  • Also reconciles "closed" PRs in pullRequests that have a non-null mergedAt
    in their merged record (mislabelled by the GitHub API at sync time).
  • Fixed a secondary consequence: evidenceCompleteness ratios now use only
    pullRequests-sourced records as the denominator — merged-only records are
    never eligible for detail sync and must not dilute filesCompletenessRatio.
  • Adds 4 targeted tests covering: the exact reproduction case from [Bug]: buildRepoOutcomePatterns ignores the recent_merged_pull_requests table — merge-rate analysis runs almost without merged PRs, falsely flagging healthy repos as "high closure risk" #312,
    no-double-count invariant, mislabelled-closed reconciliation, and the guard
    that mergedAt: null merged records are not incorrectly reconciled.

Related to #82.

Does not close it, as it's the parent issue/roadmap.

Scope

  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; global coverage stays at or above 97% for lines, statements, functions, and branches (aim for 98%+ branch coverage locally so CI variance does not fail near the threshold)
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • actionlint, test:workers, build:mcp, test:mcp-pack, ui:*: change is
    confined to src/signals/engine.ts (pure signal logic, no API routes, no MCP
    handlers, no UI). These gates are not impacted by this diff.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (not applicable — no auth or session changes)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (not applicable — no schema or contract changes; buildRepoOutcomePatterns is an internal signal builder)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (not applicable — no UI changes)
  • Visible UI changes include screenshots or a short recording. (not applicable)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • maintainerLane defaults to false for merged-only records because
    RecentMergedPullRequestRecord carries no authorAssociation. This is the
    conservative/correct assumption: the merged history table is populated by the
    outside-contributor backfill path. Maintainer-lane PRs tracked in
    pullRequests are still classified correctly via their authorAssociation.
  • changedLineCount is 0 for merged-only records (no PullRequestFileRecord
    rows exist for them). Size-by-file-count bucketing still works via filePaths.
  • Coverage: 798/799 tests pass (1 pre-existing skip), typecheck clean,
    97.02% branches / 98.99% statements globally.

@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.

@enjoyandlove

A few notes:

  • The core bug is real: recent_merged_pull_requests has to participate in repo outcome analysis or healthy repos get falsely flagged as low-merge/high-closure-risk.
  • The evidence-completeness denominator change is also important because merged-only rows are not detail-sync eligible.
  • The remaining blocker is maintainer-lane accuracy. Merged-only records are currently added with maintainerLane: false and authorRole: "first_time_or_external", which can inflate outside-contributor merge rates when the merged history includes owner/member/collaborator work.

Required changes:

  • Derive maintainer association from available merged-row payload when present, or exclude unknown-association merged-only rows from outside-contributor merge-rate calculations.
  • Add a regression where a merged-only OWNER/MEMBER/COLLABORATOR PR does not improve outside-contributor merge-rate signals.
  • Keep the evidence-completeness denominator fix.

Validation expected:

  • Run test/unit/repo-outcome-patterns.test.ts.
  • Run the full validate pipeline after the association fix.

@dosubot dosubot Bot added size:L and removed size:M labels Jun 2, 2026
@enjoyandlove

Copy link
Copy Markdown
Contributor Author

Hi, @JSONbored
Could you please review this PR?

@JSONbored
JSONbored self-requested a review June 3, 2026 05:27

@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.

@enjoyandlove this still needs the maintainer-lane accuracy fix before merge.

A few notes:

  • Including recent merged PRs is the right direction for #312.
  • The issue is that merged-only records should not inflate outside-contributor success patterns when author/maintainer lane cannot be classified safely.
  • The current title is also truncated.

Required changes:

  • Retitle the PR to a complete scoped title, for example fix(signals): include recent merged PRs in outcome patterns.
  • Fix the maintainer-lane classification for merged-only records.
  • Make the fallback conservative when author association is unknown.

Validation expected:

  • Add/keep focused tests for maintainer-authored, external-author, and unknown-author merged PR records.
  • Re-run the targeted signal tests and the normal CI gate.

@superagent-security superagent-security Bot added pr:flagged PR flagged for review by security analysis. and removed pr:flagged PR flagged for review by security analysis. labels 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.

@enjoyandlove this is closer, but cleanup is still needed.

A few notes:

  • Including recent merged PRs in outcome patterns is the right fix for #312.
  • The latest code is more conservative about unknown/maintainer associations.
  • The title is still truncated and the branch still includes the generated extension zip artifact.

Required changes:

  • Retitle to fix(signals): include recent merged PRs in outcome patterns.
  • Remove apps/gittensory-ui/public/downloads/gittensory-extension.zip.
  • Fix merge conflicts.
  • Re-request review after artifact cleanup.

Validation expected:

  • Keep CI green.
  • Re-run test/unit/repo-outcome-patterns.test.ts.

Signed-off-by: Chan <101856681+enjoyandlove@users.noreply.github.com>
@dosubot dosubot Bot added size:M and removed size:L labels Jun 3, 2026
@dosubot dosubot Bot added size:S and removed size:M labels Jun 3, 2026
@JSONbored
JSONbored self-requested a review June 4, 2026 06:04

@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.

approved, merging.

@dosubot dosubot Bot added the lgtm label Jun 4, 2026
@JSONbored
JSONbored merged commit c47236e into JSONbored:main Jun 4, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 4, 2026
@JSONbored JSONbored changed the title fix(signals): include recent_merged_pull_requests in outcome pattern … fix(signals): include recent_merged_pull_requests in outcome pattern Jun 4, 2026
@enjoyandlove

Copy link
Copy Markdown
Contributor Author

Thank you for merging this PR

@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

Development

Successfully merging this pull request may close these issues.

2 participants