Skip to content

feat(signals): flag open-vs-open PR file-path collisions - #2654

Merged
JSONbored merged 1 commit into
mainfrom
claude/sweet-fermat-d33c19
Jul 3, 2026
Merged

feat(signals): flag open-vs-open PR file-path collisions#2654
JSONbored merged 1 commit into
mainfrom
claude/sweet-fermat-d33c19

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • PullRequestRecord (open PRs) never carried changedFiles, unlike RecentMergedPullRequestRecord. prItem() in src/signals/engine.ts built collision items for open PRs with no path data, so buildCollisionReport's existing termOverlap scoring (which already tokenizes changedFiles for merged PRs via recentMergedItem) only ever saw title/label text for open work. Two independently-open PRs touching the same file got zero warning from the "Related work" panel.
  • This PR populates changedFiles on PullRequestRecord/prItem() from the pull_request_files cache (a plain D1 read populated by the routine detail-sync backfill, not a live GitHub call), reusing the exact scoring machinery already shipped for merged PRs. Wiring into the live per-PR review path is flag-gated (GITTENSORY_OPEN_PR_FILE_COLLISION, default false) since the collision/preflight path is shared by several other gates.
  • Added a same-author guard in buildCollisionReport: a contributor's own follow-up/stacked PR sharing a file with their still-open prior PR is legitimate iteration, not duplicate work, and must not be flagged. The guard is scoped specifically to matches driven by path terms — pre-existing title/label-overlap behavior between same-author items (already shipped and tested, e.g. the bounded-PR-sampling regression tests) is unchanged.
  • Closes Open PRs carry no file-path metadata, so the collision engine misses open-vs-open path overlap #2653.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • 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; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • 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

Ran the full npm run test:ci chain plus npm audit --audit-level=moderate locally — all green. Verified branch coverage on every changed line in src/signals/engine.ts and src/queue/processors.ts directly against the v8 coverage JSON (both sides of every new conditional/ternary/default-param hit). New tests: unit coverage for the path-overlap scoring + same-author guard (including the merged-PR path) in test/unit/signals-v2.test.ts, unit coverage for the new enrichOpenPullRequestsWithChangedFiles DB helper (including its early-return/fail-safe branches) and two webhook-driven integration tests proving the flag-on and flag-off (byte-identical default) live-review behavior in test/unit/queue.test.ts.

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.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

No UI/auth/CORS/session surface touched — backend-only, no UI Evidence needed.

Notes

  • New env var GITTENSORY_OPEN_PR_FILE_COLLISION defaults to "false" in wrangler.jsonc; the live-wiring change is byte-identical until it's explicitly turned on.

Open PRs carried no changedFiles, so buildCollisionReport's existing
termOverlap scoring only ever saw title/label text for open work,
silently missing two independently-open PRs touching the same file.
Populate changedFiles on PullRequestRecord from the pull_request_files
cache (flag-gated GITTENSORY_OPEN_PR_FILE_COLLISION, default off) and
add a same-author guard so a contributor's own follow-up PR sharing a
file with their prior PR is never flagged as a collision.
@dosubot dosubot Bot added the size:M label Jul 3, 2026
@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-03 00:55:54 UTC

8 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI green · blocked

⏸️ Suggested Action - Manual Review

  • Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.

Review summary
The change wires cached open-PR file paths into collision scoring behind a global env flag, and the core path is coherent: open PR records gain optional `changedFiles`, `prItem()` forwards it, and the collision scorer can suppress same-author path-only matches while preserving title/label overlap behavior. The visible tests cover the main flag-on/flag-off path plus same-author and recently-merged variants. I do not see a must-fix runtime defect in the diff, but there are a couple of contract/operational edges worth tightening before relying on this broadly.

Nits — 6 non-blocking
  • nit: `src/queue/processors.ts:5821` overwrites `candidate.changedFiles` whenever the cache has rows, despite the comment saying only records with no existing `changedFiles` are overwritten; preserve an already-populated value or update the contract comment.
  • nit: `src/env.d.ts:310` says a contributor's own two PRs sharing a file are "never" flagged, but `src/signals/engine.ts:867` still allows same-author pairs when title/label terms clear the threshold without paths, so the env docs should match the narrower path-only guard.
  • nit: `src/queue/processors.ts:5808` should confirm the detail-sync producer prunes stale `pull_request_files` rows for open PRs; otherwise this new scorer can keep flagging a file after a PR no longer touches it.
  • In `src/queue/processors.ts:5821`, change the map branch to keep `candidate.changedFiles` when it is already defined, then add a unit case proving pre-resolved paths are not replaced by cache rows.
  • In `src/env.d.ts:310`, mirror the engine behavior: same-author PR-shaped pairs are skipped only when file paths are the terms that push the overlap over the threshold.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

  • Possible leaked secret in the diff (generic_secret_assignment) — Remove the secret from the diff, rotate the exposed credential, then re-run the gate.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #2653
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:M; 1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 61 registered-repo PR(s), 52 merged, 506 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 61 PR(s), 506 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 61 PR(s), 506 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jul 3, 2026
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.09%. Comparing base (33b4cbf) to head (8b11d42).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2654   +/-   ##
=======================================
  Coverage   96.09%   96.09%           
=======================================
  Files         237      237           
  Lines       26474    26489   +15     
  Branches     9602     9609    +7     
=======================================
+ Hits        25440    25455   +15     
  Misses        425      425           
  Partials      609      609           
Files with missing lines Coverage Δ
src/queue/processors.ts 92.44% <100.00%> (+0.04%) ⬆️
src/signals/engine.ts 97.71% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jul 3, 2026
@JSONbored
JSONbored merged commit 68d9f6a into main Jul 3, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/sweet-fermat-d33c19 branch July 3, 2026 01:14
statxc pushed a commit to statxc/gittensory that referenced this pull request Jul 3, 2026
…ubmissions (JSONbored#2586)

* fix(review): stop the surface lane from auto-closing clean registry submissions

Two bugs in the registry surface-review lane were closing structurally
clean registry submissions:

1. classifyRegistryPrScope already approves a genuine debut-provider
   companion file riding alongside an entry submission (isAllowed
   matches providerFilePattern), but runSurfaceReview threw that away
   and routed any companion file straight to manual review. It now
   validates the companion via the spec's assessProviderEntry and
   combines it with the entry's own assessment: merge only when both
   sides are clean, close if either is invalid. artifactPattern
   companions (generated build output) are still allowed as-is with no
   validation attempted. The entry/base/companion fetches run
   concurrently instead of sequentially.

2. A duplicate_pr_risk finding (severity "warning") escalated into a
   hard blocker by duplicatePrGateMode: "block" was able to
   singlehandedly override a clean, deterministic surface-lane merge
   and force the whole PR closed via applySurfaceGate's unconditional
   union. It now downgrades to a neutral hold instead, mirroring the
   existing AI-judgment-only carve-out, and the held check-run's
   title/summary name the actual reason. This is scoped to exactly
   duplicate_pr_risk (not every warning-severity finding), since
   missing_linked_issue / self_authored_linked_issue /
   manifest_linked_issue_required / manifest_missing_tests are also
   warning-severity but block-mode-escalatable via their own
   independent maintainer-configured gate and must still close
   outright when a maintainer opts into that.

Both were confirmed live against JSONbored/metagraphed: PR JSONbored#2654 (an
entry + debut-provider companion) and PR JSONbored#2680 (a provider-only
resubmission sharing JSONbored#2654's linked issue) now resolve to merge and a
held-for-review neutral hold respectively, instead of reject/close.

* fix(review): confirm a provider companion is actually a debut before merging it

classifyRegistryPrScope identifies a companion by file path alone
(providerFilePattern), which only proves the file is shaped like a
provider submission, not that it's a genuine debut (a brand-new
provider, not an edit to one already registered). runSurfaceReview now
also fetches the companion's base content and only runs it through the
debut-provider merge/close flow when base is absent; a companion that
already exists at base routes to manual instead, since editing an
existing, unrelated provider record alongside an entry submission is a
more sensitive shape that needs a human. The entry and both companion
refs are still fetched in one concurrent round-trip.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open PRs carry no file-path metadata, so the collision engine misses open-vs-open path overlap

1 participant