Skip to content

Commit 61ab43e

Browse files
fix(engine): drop opportunity-metadata's blanket v8-ignore directives and branch-cover the scoring logic they hid (#9635)
opportunity-metadata.ts carried v8-ignore suppressions across nearly all of its decision logic -- computeMetadataPotential wholesale, the feasibility clock guard and title tiers, titlesOverlap, all five computeMetadataDupRisk branches, and six bare unexplained directives -- in a file the engine Codecov flag is supposed to grade. The rationales were self-refuting ("exercised in ranker tests": if true the directive is redundant, if false it conceals an untested branch), and stripping them proves the point: four partial branches surface under the existing ranker suites alone. Delete every directive except the opportunityMetadataInternals test-only export block's legitimate pair. No production logic changes -- the diff to the source file is 27 pure comment-line deletions; every formula, threshold, label list, and return expression is byte-for-byte identical. Direct unit tests now pin each previously-suppressed branch: label tiers and normalization, the non-finite clock guard, title-length tiers, titlesOverlap's emptiness/equality/containment- threshold/swap arms, dup-risk's self/cross-repo/overlap filters, the competition-context and timestamp ?? arms, the targetability filter, and the defensive re-parse and clamp01 guards (exercised by making the underlying primitive disagree, the only way without a logic change). Regression tests land in BOTH graded suites: the engine package's own node:test suite (the `engine` Codecov flag's coverage run) and the root vitest unit suite (the `backend` flag). Under each flag's own lcov the file now reports zero uncovered lines and zero partial branches outside the retained internals block. Closes #9616 Co-authored-by: rsnetworkinginc <rsnetworkinginc@users.noreply.github.com>
1 parent c086363 commit 61ab43e

3 files changed

Lines changed: 405 additions & 27 deletions

File tree

packages/loopover-engine/src/opportunity-metadata.ts

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,15 @@ const NEGATIVE_LABELS = Object.freeze([
4343
]);
4444

4545
function clamp01(value: number): number {
46-
/* v8 ignore next -- Defensive guard for malformed adapter input; scores are always finite in practice. */
4746
if (!Number.isFinite(value)) return 0;
4847
return Math.min(1, Math.max(0, value));
4948
}
5049

5150
function finiteNonNegativeInt(value: number): number {
52-
/* v8 ignore next -- Defensive guard for malformed adapter input; counts are normalized before scoring. */
5351
if (!Number.isFinite(value)) return 0;
5452
return Math.max(0, Math.trunc(value));
5553
}
5654

57-
/* v8 ignore start -- Label/title normalization helpers are covered through exported ranker entrypoints. */
5855
function normalizeLabels(labels: readonly string[]): string[] {
5956
return labels
6057
.filter((label): label is string => typeof label === "string")
@@ -74,11 +71,9 @@ function resolveGoalSpec(repoFullName: string, context: MetadataRankContext): Mi
7471
}
7572
return DEFAULT_MINER_GOAL_SPEC;
7673
}
77-
/* v8 ignore stop */
7874

7975
const STALE_AGE_DAYS = 9999;
8076

81-
/* v8 ignore start -- Internal timestamp helpers mirror freshness semantics; exercised via exported ranker paths. */
8277
function pickMetadataTimestamp(issue: MetadataCandidateIssue): string {
8378
// Mirror freshness semantics (opportunity-freshness's pickTimestamp): only commit to a timestamp that actually
8479
// parses. Without the guard, a present-but-unparseable updatedAt shadows a valid createdAt, so issueAgeDays
@@ -101,51 +96,40 @@ function issueAgeDays(issue: MetadataCandidateIssue, nowMs: number): number {
10196
if (!Number.isFinite(parsed)) return STALE_AGE_DAYS;
10297
return Math.max(0, Math.floor((nowMs - parsed) / 86_400_000));
10398
}
104-
/* v8 ignore stop */
10599

106100
/**
107101
* Estimate reward potential from issue labels alone. Explicitly negative labels collapse the score; common
108102
* contribution labels raise it; everything else keeps a neutral baseline.
109103
*/
110-
/* v8 ignore start -- Metadata heuristics are exercised end-to-end in test/unit/miner-opportunity-ranker.test.ts. */
111104
export function computeMetadataPotential(issue: { labels: readonly string[] }): number {
112105
const labels = normalizeLabels(issue.labels);
113-
/* v8 ignore next -- Terminal labels short-circuit to zero potential; exercised in ranker tests. */
114106
if (labels.some((label) => NEGATIVE_LABELS.includes(label))) return 0;
115107
let score = 0.45;
116-
/* v8 ignore next -- Neutral metadata keeps the baseline when no contribution labels are present. */
117108
if (labels.some((label) => POSITIVE_LABELS.includes(label))) score += 0.35;
118-
/* v8 ignore next -- Bug/refactor bonuses are additive; neutral-only labels keep the baseline score. */
119109
if (labels.includes("bug")) score += 0.1;
120-
/* v8 ignore next */
121110
if (labels.includes("refactor")) score += 0.05;
122111
return clamp01(score);
123112
}
124-
/* v8 ignore stop */
125113

126114
/**
127115
* Estimate achievability from metadata-only cues: lower discussion load and fresher issues score higher.
128116
*/
129117
export function computeMetadataFeasibility(issue: MetadataCandidateIssue, nowMs: number): number {
130-
/* v8 ignore next -- Ranker callers inject a finite epoch; malformed clocks degrade to zero feasibility. */
131118
if (!Number.isFinite(nowMs)) return 0;
132119
const comments = finiteNonNegativeInt(issue.commentsCount);
133120
const commentScore = clamp01(1 - comments / 25);
134121
const ageDays = issueAgeDays(issue, nowMs);
135122
const ageScore = clamp01(Math.exp(-ageDays / 45));
136123
const titleLength = normalizeTitle(issue.title).length;
137-
/* v8 ignore start -- Title-length tiers are covered through ranker integration tests. */
138124
let titleScore = 0.4;
139125
if (titleLength >= 8) {
140126
titleScore = 1;
141127
} else if (titleLength >= 4) {
142128
titleScore = 0.7;
143129
}
144-
/* v8 ignore stop */
145130
return clamp01(commentScore * 0.45 + ageScore * 0.35 + titleScore * 0.2);
146131
}
147132

148-
/* v8 ignore start -- Title overlap helper is exercised through computeMetadataDupRisk. */
149133
function titlesOverlap(left: string, right: string): boolean {
150134
if (!left || !right) return false;
151135
if (left === right) return true;
@@ -157,7 +141,6 @@ function titlesOverlap(left: string, right: string): boolean {
157141
}
158142
return longer.includes(shorter) && shorter.length >= 12;
159143
}
160-
/* v8 ignore stop */
161144

162145
/* v8 ignore start -- Test-only export surface for branch coverage. */
163146
export const opportunityMetadataInternals = {
@@ -177,18 +160,13 @@ export function computeMetadataDupRisk(
177160
peers: readonly MetadataCandidateIssue[],
178161
): number {
179162
const normalized = normalizeTitle(issue.title);
180-
/* v8 ignore next -- Blank titles are treated as maximum dup risk. */
181163
if (!normalized) return 1;
182164
let overlaps = 0;
183165
for (const peer of peers) {
184-
/* v8 ignore next -- Self-peer rows are skipped when scanning the shared batch list. */
185166
if (peer.issueNumber === issue.issueNumber && peer.repoFullName.trim().toLowerCase() === issue.repoFullName.trim().toLowerCase()) continue;
186-
/* v8 ignore next -- Cross-repo peers are ignored when scanning for overlap inside a batch. */
187167
if (peer.repoFullName.trim().toLowerCase() !== issue.repoFullName.trim().toLowerCase()) continue;
188-
/* v8 ignore next -- Overlap hits are counted only for same-repo peers with shared title segments. */
189168
if (titlesOverlap(normalized, normalizeTitle(peer.title))) overlaps += 1;
190169
}
191-
/* v8 ignore next -- No overlaps keeps dup risk at zero for unique titles. */
192170
if (overlaps === 0) return 0;
193171
return clamp01(overlaps / (overlaps + 1));
194172
}
@@ -201,9 +179,7 @@ export function buildMetadataRankInput(
201179
): OpportunityRankInput {
202180
const goalSpec = resolveGoalSpec(issue.repoFullName, context);
203181
const repoCompetition = computeOpportunityCompetition(
204-
/* v8 ignore next */
205182
context.highRiskDuplicateClusters ?? 0,
206-
/* v8 ignore next */
207183
context.openPullRequests ?? 0,
208184
);
209185
const batchDupRisk = computeMetadataDupRisk(issue, peers);
@@ -212,11 +188,9 @@ export function buildMetadataRankInput(
212188
feasibility: computeMetadataFeasibility(issue, context.nowMs),
213189
laneFit: computeMetadataLaneFit(issue, goalSpec),
214190
freshness: computeOpportunityFreshness(
215-
/* v8 ignore next */
216191
[{ state: "open", updatedAt: issue.updatedAt ?? null, createdAt: issue.createdAt ?? null }],
217192
context.nowMs,
218193
),
219-
/* v8 ignore next */
220194
dupRisk: clamp01(Math.max(batchDupRisk, repoCompetition)),
221195
};
222196
}
@@ -233,6 +207,5 @@ export function rankMetadataOpportunities<T extends MetadataCandidateIssue>(
233207
...candidate,
234208
...buildMetadataRankInput(candidate, targetableCandidates, context),
235209
}));
236-
/* v8 ignore next */
237210
return rankOpportunities(annotated) as Array<T & OpportunityRankInput & { rankScore: number }>;
238211
}
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
import assert from "node:assert/strict";
2+
import { test } from "node:test";
3+
4+
import {
5+
DEFAULT_MINER_GOAL_SPEC,
6+
buildMetadataRankInput,
7+
computeMetadataDupRisk,
8+
computeMetadataFeasibility,
9+
computeMetadataPotential,
10+
rankMetadataOpportunities,
11+
type MetadataCandidateIssue,
12+
type MetadataRankContext,
13+
} from "../dist/index.js";
14+
import { opportunityMetadataInternals } from "../dist/opportunity-metadata.js";
15+
16+
// #9616: direct unit tests for every branch the file's blanket `v8 ignore` directives used to hide.
17+
// The scoring formulas themselves are untouched -- these tests pin the thresholds, label bonuses, and
18+
// filters so a future change to any of them fails a test instead of vanishing behind a suppression.
19+
20+
const NOW_ISO = "2026-07-29T00:00:00Z";
21+
const NOW = Date.parse(NOW_ISO);
22+
23+
function issueFor(overrides: Partial<MetadataCandidateIssue> = {}): MetadataCandidateIssue {
24+
return {
25+
repoFullName: "acme/widgets",
26+
issueNumber: 1,
27+
title: "improve the flaky retry logic in ci",
28+
labels: [],
29+
commentsCount: 0,
30+
createdAt: NOW_ISO,
31+
updatedAt: NOW_ISO,
32+
...overrides,
33+
};
34+
}
35+
36+
function assertClose(actual: number, expected: number): void {
37+
assert.ok(Math.abs(actual - expected) < 1e-9, `${actual} !~ ${expected}`);
38+
}
39+
40+
test("computeMetadataPotential: label tiers -- negative collapse, positive/bug/refactor bonuses, neutral baseline", () => {
41+
// A negative label collapses to 0, even alongside positive ones (short-circuit).
42+
assert.equal(computeMetadataPotential({ labels: ["blocked"] }), 0);
43+
assert.equal(computeMetadataPotential({ labels: ["help wanted", "wontfix"] }), 0);
44+
// Neutral-only labels keep the 0.45 baseline.
45+
assertClose(computeMetadataPotential({ labels: ["chore"] }), 0.45);
46+
assertClose(computeMetadataPotential({ labels: [] }), 0.45);
47+
// Positive label bonus.
48+
assertClose(computeMetadataPotential({ labels: ["help wanted"] }), 0.8);
49+
// Bug and refactor bonuses, separately and together.
50+
assertClose(computeMetadataPotential({ labels: ["bug"] }), 0.55);
51+
assertClose(computeMetadataPotential({ labels: ["refactor"] }), 0.5);
52+
assertClose(computeMetadataPotential({ labels: ["bug", "refactor"] }), 0.6);
53+
assertClose(computeMetadataPotential({ labels: ["good first issue", "bug", "refactor"] }), 0.95);
54+
});
55+
56+
test("computeMetadataPotential: labels are normalized -- trims, lowercases, drops non-strings and blanks", () => {
57+
assertClose(computeMetadataPotential({ labels: [" BUG "] }), 0.55);
58+
assertClose(computeMetadataPotential({ labels: ["", " ", 7, null] as never }), 0.45);
59+
});
60+
61+
test("computeMetadataFeasibility: a non-finite clock degrades to zero feasibility", () => {
62+
assert.equal(computeMetadataFeasibility(issueFor(), Number.NaN), 0);
63+
assert.equal(computeMetadataFeasibility(issueFor(), Number.POSITIVE_INFINITY), 0);
64+
});
65+
66+
test("computeMetadataFeasibility: title-length tiers at >= 8, 4-7, and < 4 (normalized length)", () => {
67+
// Fresh issue, zero comments: commentScore 1 and ageScore 1, so the tier is the only variable.
68+
assertClose(computeMetadataFeasibility(issueFor({ title: "abcdefgh" }), NOW), 1);
69+
assertClose(computeMetadataFeasibility(issueFor({ title: "abcd" }), NOW), 0.94);
70+
assertClose(computeMetadataFeasibility(issueFor({ title: "abc" }), NOW), 0.88);
71+
// normalizeTitle collapses interior whitespace before measuring: " a b " -> "a b" (length 3).
72+
assertClose(computeMetadataFeasibility(issueFor({ title: " a b " }), NOW), 0.88);
73+
});
74+
75+
test("computeMetadataFeasibility: comment load and timestamp fallbacks", () => {
76+
// 25+ comments zero out the comment score.
77+
assertClose(computeMetadataFeasibility(issueFor({ commentsCount: 25 }), NOW), 0.35 + 0.2);
78+
// A non-finite comment count is normalized to 0, not propagated.
79+
assert.equal(
80+
computeMetadataFeasibility(issueFor({ commentsCount: Number.NaN }), NOW),
81+
computeMetadataFeasibility(issueFor({ commentsCount: 0 }), NOW),
82+
);
83+
assert.equal(
84+
computeMetadataFeasibility(issueFor({ commentsCount: -3 }), NOW),
85+
computeMetadataFeasibility(issueFor({ commentsCount: 0 }), NOW),
86+
);
87+
// No parseable timestamp at all: the stale-age sentinel zeroes the age term.
88+
assertClose(computeMetadataFeasibility(issueFor({ updatedAt: null, createdAt: null }), NOW), 0.65);
89+
// An unparseable updatedAt falls back to a valid createdAt instead of shadowing it.
90+
assertClose(computeMetadataFeasibility(issueFor({ updatedAt: "not a date", createdAt: NOW_ISO }), NOW), 1);
91+
});
92+
93+
test("computeMetadataFeasibility: a validated timestamp that later fails to re-parse hits the stale sentinel (defensive guard)", () => {
94+
// pickMetadataTimestamp validates with the same parser issueAgeDays re-parses with, so the re-parse
95+
// guard can only be exercised by making the parser itself disagree between the two calls.
96+
const realParse = Date.parse;
97+
let calls = 0;
98+
Date.parse = ((value: string) => {
99+
calls += 1;
100+
return calls === 1 ? realParse(value) : Number.NaN;
101+
}) as typeof Date.parse;
102+
try {
103+
const reparseFailed = computeMetadataFeasibility(issueFor(), NOW);
104+
const noTimestamps = computeMetadataFeasibility(issueFor({ updatedAt: null, createdAt: null }), NOW);
105+
assert.equal(reparseFailed, noTimestamps);
106+
assert.equal(calls, 2);
107+
} finally {
108+
Date.parse = realParse;
109+
}
110+
});
111+
112+
test("computeMetadataFeasibility: a non-finite intermediate score clamps to zero (clamp01 defensive guard)", () => {
113+
// Every natural input reaching clamp01 is finite, so the guard is exercised by making the age
114+
// exponential itself misbehave.
115+
const realExp = Math.exp;
116+
Math.exp = (() => Number.NaN) as typeof Math.exp;
117+
try {
118+
assertClose(computeMetadataFeasibility(issueFor(), NOW), 0.45 + 0.2);
119+
} finally {
120+
Math.exp = realExp;
121+
}
122+
});
123+
124+
test("titlesOverlap: emptiness, equality, containment threshold, and swap arms", () => {
125+
const { titlesOverlap } = opportunityMetadataInternals;
126+
assert.equal(titlesOverlap("", "anything"), false);
127+
assert.equal(titlesOverlap("anything", ""), false);
128+
assert.equal(titlesOverlap("same words", "same words"), true);
129+
// Containment counts only when the shorter title is >= 12 chars.
130+
assert.equal(titlesOverlap("fix the retry logic", "please fix the retry logic now"), true);
131+
assert.equal(titlesOverlap("short one", "short one plus context"), false);
132+
// Swap arm: left longer than right.
133+
assert.equal(titlesOverlap("please fix the retry logic now", "fix the retry logic"), true);
134+
// Long enough on both sides but no containment.
135+
assert.equal(titlesOverlap("abcdefghijklmnop", "qrstuvwxyz0123456789"), false);
136+
});
137+
138+
test("normalizeLabels / pickMetadataTimestamp / resolveGoalSpec internals cover their normalization arms", () => {
139+
const { normalizeLabels, pickMetadataTimestamp, resolveGoalSpec } = opportunityMetadataInternals;
140+
assert.deepEqual(normalizeLabels([" Bug ", "", 7, null] as never), ["bug"]);
141+
142+
assert.equal(pickMetadataTimestamp(issueFor()), NOW_ISO);
143+
assert.equal(pickMetadataTimestamp(issueFor({ updatedAt: ` ${NOW_ISO} ` })), NOW_ISO);
144+
assert.equal(pickMetadataTimestamp(issueFor({ updatedAt: "not a date", createdAt: NOW_ISO })), NOW_ISO);
145+
assert.equal(pickMetadataTimestamp(issueFor({ updatedAt: " ", createdAt: " " })), "");
146+
assert.equal(pickMetadataTimestamp(issueFor({ updatedAt: null, createdAt: "garbage" })), "");
147+
assert.equal(pickMetadataTimestamp(issueFor({ updatedAt: null, createdAt: null })), "");
148+
149+
const custom = { ...DEFAULT_MINER_GOAL_SPEC, minerEnabled: false };
150+
const context: MetadataRankContext = { nowMs: NOW, goalSpecsByRepo: { " ACME/Widgets ": custom } };
151+
assert.equal(resolveGoalSpec("acme/widgets", context), custom);
152+
assert.equal(resolveGoalSpec("other/repo", context), DEFAULT_MINER_GOAL_SPEC);
153+
assert.equal(resolveGoalSpec("acme/widgets", { nowMs: NOW }), DEFAULT_MINER_GOAL_SPEC);
154+
});
155+
156+
test("computeMetadataDupRisk: blank titles, peer filters, and overlap accumulation", () => {
157+
// A blank (or whitespace-only) title is maximum dup risk.
158+
assert.equal(computeMetadataDupRisk(issueFor({ title: "" }), []), 1);
159+
assert.equal(computeMetadataDupRisk(issueFor({ title: " " }), []), 1);
160+
// The issue's own batch row is skipped, including with repo-name case/whitespace variance.
161+
assert.equal(computeMetadataDupRisk(issueFor(), [issueFor({ repoFullName: " ACME/Widgets " })]), 0);
162+
// Same issue number in a DIFFERENT repo is not self -- it falls through to the cross-repo skip.
163+
assert.equal(computeMetadataDupRisk(issueFor(), [issueFor({ repoFullName: "other/repo" })]), 0);
164+
// Cross-repo peers never count, even with identical titles.
165+
assert.equal(computeMetadataDupRisk(issueFor(), [issueFor({ repoFullName: "other/repo", issueNumber: 9 })]), 0);
166+
// A same-repo peer with an overlapping title raises the risk: 1 overlap -> 1/2.
167+
const overlapping = issueFor({ issueNumber: 2, title: "Improve the flaky retry logic in CI for the queue" });
168+
assertClose(computeMetadataDupRisk(issueFor(), [overlapping]), 0.5);
169+
// Two overlapping peers -> 2/3.
170+
const second = issueFor({ issueNumber: 3, title: "improve the flaky retry logic in ci again" });
171+
assertClose(computeMetadataDupRisk(issueFor(), [overlapping, second]), 2 / 3);
172+
// Same-repo peers with unrelated titles keep the risk at zero.
173+
assert.equal(computeMetadataDupRisk(issueFor(), [issueFor({ issueNumber: 4, title: "completely unrelated words" })]), 0);
174+
});
175+
176+
test("buildMetadataRankInput: competition context present and absent, timestamps present and absent", () => {
177+
const withContext = buildMetadataRankInput(issueFor({ labels: ["bug"] }), [], {
178+
nowMs: NOW,
179+
highRiskDuplicateClusters: 2,
180+
openPullRequests: 4,
181+
});
182+
assertClose(withContext.potential, 0.55);
183+
assertClose(withContext.feasibility, 1);
184+
// dupRisk = max(batch 0, competition 2/4).
185+
assertClose(withContext.dupRisk, 0.5);
186+
assert.ok(withContext.laneFit >= 0 && withContext.laneFit <= 1);
187+
assert.ok(withContext.freshness >= 0 && withContext.freshness <= 1);
188+
189+
const bare = buildMetadataRankInput(issueFor({ updatedAt: null, createdAt: null }), [], { nowMs: NOW });
190+
assert.equal(bare.dupRisk, 0);
191+
assert.ok(bare.freshness >= 0 && bare.freshness <= 1);
192+
});
193+
194+
test("rankMetadataOpportunities: ranks targetable candidates and drops repos whose goal spec opts out", () => {
195+
const strong = issueFor({ issueNumber: 10, title: "add retry backoff to the queue worker", labels: ["help wanted", "bug"] });
196+
const weak = issueFor({ issueNumber: 11, title: "abc", labels: ["chore"], commentsCount: 40, updatedAt: null, createdAt: null });
197+
const ranked = rankMetadataOpportunities([strong, weak], { nowMs: NOW });
198+
assert.equal(ranked.length, 2);
199+
assert.equal(typeof ranked[0]?.rankScore, "number");
200+
assert.ok((ranked[0]?.rankScore ?? 0) >= (ranked[1]?.rankScore ?? 0));
201+
assert.equal(ranked[0]?.issueNumber, 10);
202+
203+
const optedOut = rankMetadataOpportunities([strong, issueFor({ repoFullName: "other/repo", issueNumber: 12 })], {
204+
nowMs: NOW,
205+
goalSpecsByRepo: { "acme/widgets": { ...DEFAULT_MINER_GOAL_SPEC, minerEnabled: false } },
206+
});
207+
assert.deepEqual(optedOut.map((candidate) => candidate.issueNumber), [12]);
208+
});

0 commit comments

Comments
 (0)