Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions packages/loopover-miner/docs/contribution-profile.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,41 @@ The profile is cached in a local SQLite store keyed by repo, mirroring the miner
- **#6796 (extraction):** populates each `SignalRule` from labels + `CONTRIBUTING.md` (root and `.github/`) +
PR template + agent docs, setting `confidence`/`provenance` per the findings above.
- **#6797 (cache + doctor):** the `miner_contribution_profile` SQLite store with the TTL above.
- **#6798 (`discover` wiring):** reads the profile's eligibility/exclusion rules and the runtime assignee check
to filter candidate issues, weighting each by its `confidence`.
- **#6798 (`discover` wiring):** reads the profile's eligibility/exclusion rules to filter candidate issues —
see the section below for the landed behavior.

## Discover eligibility filtering (#6798)

`loopover-miner discover` now filters candidate issues through each target repo's `ContributionProfile` before
ranking and enqueueing, so it no longer surfaces work a repo's own conventions would reject. The decision logic
is `filterCandidatesByProfiles` (`contribution-profile-filter.js`), a pure partition of candidates into `kept`
and `excluded` (each excluded entry carries a `reason`).

**Safe-default posture — the load-bearing rule.** Filtering activates for a repo **only** when its profile has
a trustworthy eligibility signal (`eligibilityLabels.confidence === "explicit"`). A repo with no profile, or a
low-confidence / empty one — a repo whose conventions AMS simply couldn't read — keeps **every** candidate. A
weak profile can never cause AMS to silently skip real, eligible work. On top of that, the default resolver
does no profile work at all without a GitHub token (it can't read a taxonomy reliably unauthenticated), so the
unauthenticated CLI path is byte-identical to before.

**What gets excluded, once a repo is trusted** (matched against the candidate's own labels, by the eligibility/
exclusion label _names_ the profile recorded in `provenance`):

| Reason | Meaning |
| --------------------------- | ------------------------------------------------------------------------------------------- |
| `exclusion_label` | the issue carries a label the profile marks maintainer-only / off-limits |
| `missing_eligibility_label` | the repo has an eligibility convention and the issue carries none of its eligibility labels |
| `conflicting_signals` | the issue carries **both** an eligibility and an exclusion label — **exclusion wins** |

**Conflicting signals resolve conservatively:** an issue that is both eligibility-labelled and exclusion-labelled
is excluded, because a maintainer marking it off-limits outranks its also being help-wanted — better to skip
than to attempt work the repo's own gate would reject.

**Assignee exclusion is not yet applied here.** The candidate objects that flow through `discover` carry label
names but not assignees (`opportunity-fanout.js`'s `normalizeIssue` drops them), and `ContributionAssigneeRuntimeCheck`
is deliberately a runtime concern rather than a profile field. Threading assignees through the fan-out is a
follow-up; this PR scopes filtering to labels, which is the primary eligibility signal the #6794 inventory found.

The excluded set (repo + issue + reason) is surfaced in both the `--json` output (`result.excluded`) and the
plain-text summary (an `excluded (eligibility): N` block), so a human running `discover` sees exactly what AMS
inferred and why each candidate was skipped.
20 changes: 20 additions & 0 deletions packages/loopover-miner/lib/contribution-profile-filter.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { ContributionProfile } from "./contribution-profile.js";

export const ELIGIBILITY_EXCLUSION_REASONS: {
readonly EXCLUSION_LABEL: "exclusion_label";
readonly MISSING_ELIGIBILITY_LABEL: "missing_eligibility_label";
readonly CONFLICTING_SIGNALS: "conflicting_signals";
};

export type EligibilityExclusion<T> = {
candidate: T;
reason:
"exclusion_label" | "missing_eligibility_label" | "conflicting_signals";
};

export function filterCandidatesByProfiles<
T extends { repoFullName: string; labels?: string[] },
>(
candidates: T[],
profilesByRepo: Map<string, ContributionProfile>,
): { kept: T[]; excluded: EligibilityExclusion<T>[] };
89 changes: 89 additions & 0 deletions packages/loopover-miner/lib/contribution-profile-filter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Eligibility filtering of discover candidates against a ContributionProfile (#6798). Pure: given the candidate
// list and a per-repo profile map, it partitions candidates into kept + excluded-with-reason. No fetching, no
// side effects — discover-cli.js resolves the profiles and renders the result; this owns only the decision.
//
// SAFE-DEFAULT POSTURE (the load-bearing requirement): filtering activates ONLY when a repo's profile has a
// trustworthy eligibility signal (eligibilityLabels.confidence === "explicit"). A repo with no profile, or a
// low-confidence/empty one — a repo whose conventions AMS simply couldn't read — has EVERY candidate kept, so a
// weak profile can never cause AMS to silently skip real, eligible work.

/** Why a candidate was excluded. */
export const ELIGIBILITY_EXCLUSION_REASONS = Object.freeze({
/** The issue carries a label the profile identified as maintainer-only / off-limits. */
EXCLUSION_LABEL: "exclusion_label",
/** The repo has a trustworthy eligibility convention, and the issue carries none of its eligibility labels. */
MISSING_ELIGIBILITY_LABEL: "missing_eligibility_label",
/** The issue carries BOTH an eligibility and an exclusion label — conflicting signals; exclusion wins. */
CONFLICTING_SIGNALS: "conflicting_signals",
});

/** The actual repo label names a signal rule was derived from (its provenance details), lowercased for match. */
function labelNamesFromRule(rule) {
const names = new Set();
for (const entry of rule?.provenance ?? []) {
if (typeof entry?.detail === "string")
names.add(entry.detail.toLowerCase());
}
return names;
}

/** Does the candidate carry any label whose name is in `names`? Case-insensitive. */
function candidateHasAnyLabel(candidate, names) {
if (names.size === 0) return false;
for (const label of candidate?.labels ?? []) {
if (typeof label === "string" && names.has(label.toLowerCase()))
return true;
}
return false;
}

/**
* Partition candidates into kept + excluded against per-repo ContributionProfiles.
*
* @param {Array<{ repoFullName: string, labels?: string[] }>} candidates the fanned-out discover candidates
* @param {Map<string, import("./contribution-profile.js").ContributionProfile>} profilesByRepo profile per repoFullName
* @returns {{ kept: object[], excluded: Array<{ candidate: object, reason: string }> }}
*/
export function filterCandidatesByProfiles(candidates, profilesByRepo) {
const kept = [];
const excluded = [];
for (const candidate of candidates) {
const profile = profilesByRepo?.get(candidate.repoFullName);
// Trust gate: only an EXPLICIT eligibility signal is trustworthy enough to filter on. Anything weaker
// (absent/inferred/unknown, or no profile at all) keeps every candidate — the safe default.
if (profile?.eligibilityLabels?.confidence !== "explicit") {
kept.push(candidate);
continue;
}
const eligibilityNames = labelNamesFromRule(profile.eligibilityLabels);
const exclusionNames = labelNamesFromRule(profile.exclusionLabels);
const hasEligibility = candidateHasAnyLabel(candidate, eligibilityNames);
const hasExclusion = candidateHasAnyLabel(candidate, exclusionNames);
if (hasExclusion && hasEligibility) {
// Conservative resolution for conflicting signals: exclusion wins. A maintainer marking an issue
// off-limits outranks its also carrying an eligibility label — better to skip than to attempt work the
// repo's own gate would reject.
excluded.push({
candidate,
reason: ELIGIBILITY_EXCLUSION_REASONS.CONFLICTING_SIGNALS,
});
continue;
}
if (hasExclusion) {
excluded.push({
candidate,
reason: ELIGIBILITY_EXCLUSION_REASONS.EXCLUSION_LABEL,
});
continue;
}
if (!hasEligibility) {
excluded.push({
candidate,
reason: ELIGIBILITY_EXCLUSION_REASONS.MISSING_ELIGIBILITY_LABEL,
});
continue;
}
kept.push(candidate);
}
return { kept, excluded };
}
33 changes: 31 additions & 2 deletions packages/loopover-miner/lib/discover-cli.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,23 @@ export type DiscoverFanOutSummary = {
};

/** The subset of a ranked entry that `renderDiscoverSummary` reads for its top-candidates listing. */
export type DiscoverRankedEntry = Pick<RankedCandidateIssue, "repoFullName" | "issueNumber" | "title" | "rankScore">;
export type DiscoverRankedEntry = Pick<
RankedCandidateIssue,
"repoFullName" | "issueNumber" | "title" | "rankScore"
>;

export type DiscoverResult = {
fanOutCount: number;
warnings: CandidateIssueWarning[];
rateLimitRemaining: number | null;
rateLimitResetAt: string | null;
ranked: DiscoverRankedEntry[];
/** Candidates the eligibility filter dropped, each with the repo/issue and the reason (#6798). */
excluded?: Array<{
repoFullName: string;
issueNumber: number;
reason: string;
}>;
/** True when ranking fell back to the built-in default goal spec because no per-tenant spec was supplied (#4784). */
usedDefaultGoalSpec?: boolean;
enqueueSummary: EnqueueRankedDiscoverySummary;
Expand Down Expand Up @@ -89,12 +98,32 @@ export type RunDiscoverOptions = {
* to (never instead of) the plain exit-code return -- mirrors `RunAttemptOptions.onResult`. Never fires on a
* parse-error/unexpected-error `reportCliFailure` branch, matching runAttempt's own asymmetry (#6522). */
onResult?: (result: DiscoverResult) => void;
/** Resolve each candidate repo's ContributionProfile for eligibility filtering (#6798). Defaults to
* resolveContributionProfilesForDiscover; injectable so tests avoid the network. */
resolveContributionProfiles?: (
repoFullNames: string[],
ctx: { githubToken?: string; apiBaseUrl?: string; nowMs?: number },
) => Promise<Map<string, unknown>>;
};

export function resolveContributionProfilesForDiscover(
repoFullNames: string[],
ctx?: {
githubToken?: string;
apiBaseUrl?: string;
nowMs?: number;
initCache?: unknown;
extract?: unknown;
},
): Promise<Map<string, unknown>>;

export function parseDiscoverArgs(args: string[]): ParsedDiscoverArgs;

export function sanitizeDiscoverDisplayText(value: unknown): string;

export function renderDiscoverSummary(result: DiscoverResult): string;

export function runDiscover(args: string[], options?: RunDiscoverOptions): Promise<number>;
export function runDiscover(
args: string[],
options?: RunDiscoverOptions,
): Promise<number>;
77 changes: 75 additions & 2 deletions packages/loopover-miner/lib/discover-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import { initPolicyVerdictCacheStore } from "./policy-verdict-cache.js";
import { enqueueRankedDiscovery } from "./portfolio-discovery.js";
import { initPortfolioQueueStore } from "./portfolio-queue.js";
import { initRankedCandidatesStore } from "./ranked-candidates.js";
import { extractContributionProfile } from "./contribution-profile-extract.js";
import { initContributionProfileCache } from "./contribution-profile-cache.js";
import { filterCandidatesByProfiles } from "./contribution-profile-filter.js";
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";

const DISCOVER_USAGE =
Expand Down Expand Up @@ -124,6 +127,14 @@ export function renderDiscoverSummary(result) {
if (result.enqueueSummary.skippedBelowMinRank > 0) {
lines.push(`skipped (below min rank): ${result.enqueueSummary.skippedBelowMinRank}`);
}
// #6798: surface what the eligibility filter dropped and why, so a human sees AMS's inference.
const excluded = result.excluded ?? [];
if (excluded.length > 0) {
lines.push(`excluded (eligibility): ${excluded.length}`);
for (const entry of excluded.slice(0, 10)) {
lines.push(` ${entry.repoFullName}#${entry.issueNumber} ${entry.reason}`);
}
}
// Make the fall-back to loopover's built-in rubric explicit instead of silent (#4784): when no per-tenant goal
// spec is supplied, lane fit reflects loopover's defaults, not the target repo's own conventions.
if (result.usedDefaultGoalSpec) {
Expand All @@ -143,6 +154,41 @@ export function renderDiscoverSummary(result) {
return lines.join("\n");
}

/**
* Default per-repo ContributionProfile resolver (#6798): reads the local cache and, on a miss/stale entry,
* extracts a fresh profile and caches it. Returns a Map keyed by repoFullName.
*
* WITHOUT a github token this returns an empty map and does no network work at all — AMS can't reliably read a
* repo's label taxonomy/docs unauthenticated (rate limits), so it safe-defaults to no eligibility filtering.
* That also keeps callers that don't supply a token (the common CLI path, and every test) hermetic.
*
* @param {string[]} repoFullNames unique repos among the fanned-out candidates
* @param {{ githubToken?: string, apiBaseUrl?: string, nowMs?: number, initCache?: typeof initContributionProfileCache, extract?: typeof extractContributionProfile }} ctx
* @returns {Promise<Map<string, object>>}
*/
export async function resolveContributionProfilesForDiscover(repoFullNames, ctx = {}) {
const profiles = new Map();
if (!ctx.githubToken) return profiles;
const initCache = ctx.initCache ?? initContributionProfileCache;
const extract = ctx.extract ?? extractContributionProfile;
const cache = initCache();
try {
for (const repoFullName of repoFullNames) {
const cached = cache.get(repoFullName, ctx.nowMs);
if (cached && !cached.stale) {
profiles.set(repoFullName, cached.profile);
continue;
}
const profile = await extract(repoFullName, { githubToken: ctx.githubToken, apiBaseUrl: ctx.apiBaseUrl });
cache.put(profile, ctx.nowMs);
profiles.set(repoFullName, profile);
}
} finally {
cache.close();
}
return profiles;
}

export async function runDiscover(args, options = {}) {
const parsed = parseDiscoverArgs(args);
if ("error" in parsed) {
Expand All @@ -162,6 +208,9 @@ export async function runDiscover(args, options = {}) {
const searchTargets = options.searchCandidateIssuesWithSummary ?? searchCandidateIssuesWithSummary;
const rankIssues = options.rankCandidateIssuesWithSummary ?? rankCandidateIssuesWithSummary;
const enqueue = options.enqueueRankedDiscovery ?? enqueueRankedDiscovery;
// Eligibility filtering (#6798): resolve each candidate repo's ContributionProfile and drop candidates the
// repo's own conventions would reject, BEFORE ranking. Safe by default -- see resolveContributionProfilesForDiscover.
const resolveProfiles = options.resolveContributionProfiles ?? resolveContributionProfilesForDiscover;

// #4847: fetch + rank are read-only GitHub GETs and pure local computation, so a dry run still does them for
// real (that's the useful "what would this discover?" output) -- but it never opens any local store (portfolio
Expand All @@ -175,7 +224,12 @@ export async function runDiscover(args, options = {}) {
parsed.search !== null
? await searchTargets(parsed.search, githubToken, fanOutOptions)
: await fetchTargets(parsed.targets, githubToken, fanOutOptions);
const rankedSummary = rankIssues(fanOut.issues, {
// #6798: same eligibility filter as the real path, so a dry run shows the exact candidate set a real run
// would enqueue (and the same excluded set), rather than an unfiltered preview.
const repoFullNames = [...new Set(fanOut.issues.map((issue) => issue.repoFullName))];
const profilesByRepo = await resolveProfiles(repoFullNames, { githubToken, apiBaseUrl, nowMs: options.nowMs });
const { kept, excluded } = filterCandidatesByProfiles(fanOut.issues, profilesByRepo);
const rankedSummary = rankIssues(kept, {
nowMs: options.nowMs,
goalSpecsByRepo: options.goalSpecsByRepo,
goalSpecContentByRepo: options.goalSpecContentByRepo,
Expand All @@ -189,6 +243,11 @@ export async function runDiscover(args, options = {}) {
rateLimitRemaining: fanOut.rateLimitRemaining,
rateLimitResetAt: fanOut.rateLimitResetAt,
ranked: rankedSummary.issues,
excluded: excluded.map((entry) => ({
repoFullName: entry.candidate.repoFullName,
issueNumber: entry.candidate.issueNumber,
reason: entry.reason,
})),
usedDefaultGoalSpec: rankedSummary.usedDefaultGoalSpec,
enqueueSummary,
};
Expand Down Expand Up @@ -268,10 +327,17 @@ export async function runDiscover(args, options = {}) {
? await searchTargets(parsed.search, githubToken, fanOutOptions)
: await fetchTargets(parsed.targets, githubToken, fanOutOptions);

// Eligibility filter (#6798): drop candidates a target repo's own conventions would reject, before ranking.
// A repo with no trustworthy eligibility profile keeps every candidate (filterCandidatesByProfiles' safe
// default), so this never silently skips real work on a repo whose conventions AMS couldn't read.
const repoFullNames = [...new Set(fanOut.issues.map((issue) => issue.repoFullName))];
const profilesByRepo = await resolveProfiles(repoFullNames, { githubToken, apiBaseUrl, nowMs: options.nowMs });
const { kept, excluded } = filterCandidatesByProfiles(fanOut.issues, profilesByRepo);

// Pass any caller-supplied per-tenant goal specs through to the ranker so lane fit uses the tenant's
// conventions instead of silently falling back to loopover's defaults (#4784); the fallback is surfaced via
// `usedDefaultGoalSpec` below rather than hidden.
const rankedSummary = rankIssues(fanOut.issues, {
const rankedSummary = rankIssues(kept, {
nowMs: options.nowMs,
goalSpecsByRepo: options.goalSpecsByRepo,
goalSpecContentByRepo: options.goalSpecContentByRepo,
Expand All @@ -294,6 +360,13 @@ export async function runDiscover(args, options = {}) {
rateLimitRemaining: fanOut.rateLimitRemaining,
rateLimitResetAt: fanOut.rateLimitResetAt,
ranked: rankedSummary.issues,
// #6798: candidates the eligibility filter dropped, each with the repo + issue + reason, so a human sees
// what AMS inferred and why a candidate was skipped. Empty when no profile was trustworthy enough to filter.
excluded: excluded.map((entry) => ({
repoFullName: entry.candidate.repoFullName,
issueNumber: entry.candidate.issueNumber,
reason: entry.reason,
})),
usedDefaultGoalSpec: rankedSummary.usedDefaultGoalSpec,
enqueueSummary,
};
Expand Down
Loading