Skip to content

fix(miner): carry real assignees through the discovery-index supplement so the repo-owner exclusion applies#7488

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:fix/discovery-index-assignees-7442
Jul 20, 2026
Merged

fix(miner): carry real assignees through the discovery-index supplement so the repo-owner exclusion applies#7488
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
xfodev:fix/discovery-index-assignees-7442

Conversation

@xfodev

@xfodev xfodev commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #7442. #7040 added an always-on safeguard — contribution-profile-filter.ts's isAssignedToRepoOwner excludes any discover candidate assigned to its own repo's owner, "unconditionally, for EVERY candidate." But discover-cli.ts's supplementWithDiscoveryIndex (the hosted discovery-index path, #7168) unconditionally stubbed assignees: [] on every index-sourced candidate, because assignees wasn't part of the hosted contract. isAssignedToRepoOwner reads candidate.assignees ?? [] — an always-empty array means it could never detect an owner-assignment for a discovery-index candidate, so the exclusion silently no-oped for that entire source (while the label-based rules kept working, since labels is carried through). discover-cli.ts runs filterCandidatesByProfiles over the index-supplemented rows in both the dry-run and real-run paths.

Fix

  • Contract (packages/loopover-engine/src/discovery-index-contract.ts): added an optional assignees?: readonly string[] field to DiscoveryIndexCandidate and normalized it in normalizeDiscoveryIndexCandidate with the exact same string-filtering/trimming as labels. Kept optional (unlike labels) so a client can distinguish "assignees were never served" (older server build → field omitted) from "served, and empty" — the former must fall back to [] at the merge site, not silently skip the filter. The request side carries no candidate data, so only the candidate normalizer changes.
  • Client (packages/loopover-miner/lib/discover-cli.ts): supplementWithDiscoveryIndex now copies the real assignees through ([...(candidate.assignees ?? [])]), falling back to [] only when the served response genuinely omitted the field. isAssignedToRepoOwner's own logic is untouched — the fix is getting real assignee data to it. The direct GitHub fan-out path is unchanged (it already carried real assignees).

The change is additive and backward-compatible: an older discovery-index server that doesn't populate assignees still yields [] (fail-open, exactly as today). Making the exclusion take live effect for index candidates needs the hosted packages/discovery-index server to start emitting assignees in its query response — a mechanical maintainer-side fast-follow (the labels field already proves this exact plumbing end-to-end), out of scope for this miner-side/contract PR per the issue's own scoping note.

Tests

  • Contract: normalizeDiscoveryIndexCandidate normalizes assignees like labels (drops non-strings/blanks), keeps an explicitly-served [], and omits the field entirely when the response never carries it.
  • discover-cli (end-to-end via real runDiscover with an injected queryDiscoveryIndex): a discovery-index candidate assigned to its own repo owner is now excluded by filterCandidatesByProfiles (only the direct fan-out issue survives); and a response that omits assignees falls back to [] and still runs the filter (candidate kept, not skipped), proving the fail-safe.

Every changed line + branch in both files is covered (verified via --coverage), so codecov/patch holds; all existing discovery-index-contract and miner-discover-cli tests pass unchanged.

Validation

  • git diff --check
  • npm run typecheck (root tsc --noEmit) and npm run build:miner (recompiled discover-cli.js, no drift) green
  • npm run test:coverage on the affected suite: 100% of changed lines + branches covered; npm run test:miner-pack (no .ts leak) and npm run miner:env-reference:check clean
  • Scope: the engine discovery-index contract type + the miner discover-cli client + their unit tests. No API/OpenAPI/UI/DB/wrangler surface changed.

Safety

  • No secrets, wallets, hotkeys/coldkeys, PATs, private keys, raw trust scores, private rankings, or maintainer evidence — assignees is public GitHub login data, already carried on the direct fan-out path.
  • The change only makes the owner-exclusion safeguard more consistently applied; it can't cause AMS to attempt owner-assigned work it was previously (incorrectly) picking up.
  • Auth/CORS/session: N/A. API/OpenAPI/MCP: N/A. UI: N/A. No changelog edit; no site//CNAME/lovable changes.

Closes #7442

…nt so the repo-owner exclusion applies

discover-cli's supplementWithDiscoveryIndex stubbed assignees: [] on every hosted
discovery-index candidate, so contribution-profile-filter's always-on repo-owner
exclusion (JSONbored#7040) -- which reads candidate.assignees -- could never fire for an
index-sourced candidate, silently no-opping the safeguard for that entire source.

Add an optional assignees field to DiscoveryIndexCandidate (normalized like labels,
kept optional so 'never served' is distinguishable from 'served empty') and copy the
real value through in supplementWithDiscoveryIndex, falling back to [] only when the
served response genuinely omitted it. isAssignedToRepoOwner's logic is unchanged; the
direct fan-out path already carried real assignees. Additive and backward-compatible.
Adds contract + end-to-end discover-cli regression tests.

Closes JSONbored#7442
@xfodev
xfodev requested a review from JSONbored as a code owner July 20, 2026 12:28
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.41%. Comparing base (1370f24) to head (b693055).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7488   +/-   ##
=======================================
  Coverage   91.41%   91.41%           
=======================================
  Files         717      717           
  Lines       73032    73035    +3     
  Branches    21636    21637    +1     
=======================================
+ Hits        66762    66765    +3     
  Misses       5227     5227           
  Partials     1043     1043           
Flag Coverage Δ
shard-1 30.86% <75.00%> (+<0.01%) ⬆️
shard-2 39.74% <0.00%> (-0.01%) ⬇️
shard-3 36.08% <25.00%> (-0.01%) ⬇️
shard-4 44.94% <0.00%> (-0.01%) ⬇️
shard-5 34.77% <0.00%> (-0.01%) ⬇️
shard-6 30.67% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...es/loopover-engine/src/discovery-index-contract.ts 100.00% <100.00%> (ø)
packages/loopover-miner/lib/discover-cli.ts 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 20, 2026
@loopover-orb

loopover-orb Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-20 12:45:05 UTC

5 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This closes #7442 by widening DiscoveryIndexCandidate to carry an optional assignees field, normalizing it identically to labels, and having supplementWithDiscoveryIndex copy the real value through instead of always stubbing []. The optional/undefined-vs-empty-array distinction is a deliberate and correct design for backward compatibility with older hosted builds, and the two new regression tests actually exercise the real path end-to-end (owner-assigned exclusion and omitted-field fallback), not a fabricated scenario. The docs comment in discovery-index-contract.md wasn't updated to mention the new optional assignees field, but that's a minor omission.

Nits — 5 non-blocking
  • packages/loopover-miner/docs/discovery-index-contract.md's DiscoveryIndexCandidate field list doesn't mention the new optional `assignees` field — worth a one-line addition for parity with the code.
  • The docstring in discovery-index-contract.ts:53-56 and the JS/TS comments citing 'fix(miner): discover-cli.js's discovery-index supplement stubs assignees to [], silently bypassing the #7040 repo-owner exclusion filter for index-sourced candidates #7442' as issue numbers rely on those references remaining accurate; fine as-is but slightly verbose for a one-field change.
  • discover-cli.js is a generated build artifact (sourceMappingURL diff) — verify it's actually meant to be committed/reviewed rather than produced by a build step, since diffing it doubles the reviewable surface for a small logic change.
  • Add `assignees?` to the field list in packages/loopover-miner/docs/discovery-index-contract.md for documentation parity.
  • Consider whether discover-cli.js should be gitignored/generated rather than checked in and reviewed alongside its .ts source, to avoid double-counting this PR's diff size.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7442
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 (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 55 registered-repo PR(s), 30 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor xfodev; Gittensor profile; 55 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds an optional `assignees` field to `DiscoveryIndexCandidate`, normalizes it in `normalizeDiscoveryIndexCandidate` mirroring `labels`, and updates `supplementWithDiscoveryIndex` to copy real assignees through with `[]` only as a fallback for omitted fields, leaving `isAssignedToRepoOwner` untouched as required. It also includes the two required regression tests: one proving an owner-ass

Review context
  • Author: xfodev
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, TypeScript
  • Official Gittensor activity: 55 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit e7cbb1b into JSONbored:main Jul 20, 2026
15 checks passed
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

None yet

1 participant