diff --git a/packages/loopover-miner/lib/discover-cli.ts b/packages/loopover-miner/lib/discover-cli.ts index f7014cf7f..35d59430f 100644 --- a/packages/loopover-miner/lib/discover-cli.ts +++ b/packages/loopover-miner/lib/discover-cli.ts @@ -298,8 +298,83 @@ function initDefaultSignalTrackingStore(): SignalStore { // runDiscover's real-run branch (own try/catch, degrade silently). Deliberately NOT called from the dry-run // branch: a dry run previews what a real run would do (including its own noopQueueStore for the portfolio // queue) and must not itself contribute real data to a precision report. +// #8544: bounded candidate-context capture, mirroring the ORB precedent (#8129/#8130). Caps exist so a +// pathological repo (hundreds of labels, or a label crafted to be enormous) can't grow the local event +// ledger without limit; `truncated` records THAT clamping happened so a later backtest knows the stored +// context is partial rather than treating it as the full picture. +const MAX_CAPTURED_LABELS = 50; +const MAX_CAPTURED_ASSIGNEES = 25; +const MAX_CAPTURED_STRING_CHARS = 200; + +type CapturedCandidateContext = { + labels?: string[]; + assignees?: string[]; + owner?: string; + truncated?: true; +}; + +/** #8544: clamp one string list to `max` entries and each entry to {@link MAX_CAPTURED_STRING_CHARS}, + * reporting whether either clamp actually fired. */ +function boundStringList(values: readonly string[], max: number): { values: string[]; truncated: boolean } { + const listTruncated = values.length > max; + const kept = listTruncated ? values.slice(0, max) : values; + let stringTruncated = false; + const bounded = kept.map((value) => { + if (value.length <= MAX_CAPTURED_STRING_CHARS) return value; + stringTruncated = true; + return value.slice(0, MAX_CAPTURED_STRING_CHARS); + }); + return { values: bounded, truncated: listTruncated || stringTruncated }; +} + +/** #8544: build the bounded `metadata` for one excluded candidate. Absent source fields are OMITTED entirely + * — no nulls and no empty-array placeholders, so "we captured nothing here" stays distinguishable from + * "this candidate genuinely had no labels". Returns undefined when the candidate carries no context at all, + * so pre-#8544 event shapes are reproduced byte-identically rather than gaining an empty object. */ +function buildCandidateContextMetadata(candidate: { + labels?: string[] | undefined; + assignees?: string[] | undefined; + owner?: string | undefined; +}): CapturedCandidateContext | undefined { + const metadata: CapturedCandidateContext = {}; + let truncated = false; + + if (candidate.labels !== undefined) { + const bounded = boundStringList(candidate.labels, MAX_CAPTURED_LABELS); + metadata.labels = bounded.values; + truncated ||= bounded.truncated; + } + if (candidate.assignees !== undefined) { + const bounded = boundStringList(candidate.assignees, MAX_CAPTURED_ASSIGNEES); + metadata.assignees = bounded.values; + truncated ||= bounded.truncated; + } + if (candidate.owner !== undefined) { + // Clamped directly rather than through boundStringList: a single value has no list-length arm, and + // routing it through the array helper left an unreachable `?? ""` fallback. + const clamped = candidate.owner.slice(0, MAX_CAPTURED_STRING_CHARS); + if (clamped.length < candidate.owner.length) truncated = true; + metadata.owner = clamped; + } + + if (Object.keys(metadata).length === 0) return undefined; + if (truncated) metadata.truncated = true; + return metadata; +} + async function recordEligibilityExclusionSignals( - excluded: ReadonlyArray<{ candidate: { repoFullName: string; issueNumber: number }; reason: string }>, + excluded: ReadonlyArray<{ + candidate: { + repoFullName: string; + issueNumber: number; + // #8544: already present at runtime via EligibilityExclusion's generic passthrough (see + // FilterCandidate) — read straight through, no new computation at the call site. + labels?: string[] | undefined; + assignees?: string[] | undefined; + owner?: string | undefined; + }; + reason: string; + }>, options: Pick, ): Promise { if (excluded.length === 0) return; @@ -312,12 +387,15 @@ async function recordEligibilityExclusionSignals( if (!store) return; const occurredAt = new Date(options.nowMs ?? Date.now()).toISOString(); for (const entry of excluded) { + const metadata = buildCandidateContextMetadata(entry.candidate); await store .recordRuleFired({ ruleId: entry.reason, targetKey: `${entry.candidate.repoFullName}#issue-${entry.candidate.issueNumber}`, outcome: "exclude", occurredAt, + // Spread-or-omit: a candidate with no context keeps the exact pre-#8544 event shape. + ...(metadata ? { metadata } : {}), }) .catch(() => undefined); } diff --git a/test/unit/miner-discover-cli.test.ts b/test/unit/miner-discover-cli.test.ts index 9f9403607..beaf12679 100644 --- a/test/unit/miner-discover-cli.test.ts +++ b/test/unit/miner-discover-cli.test.ts @@ -1643,12 +1643,18 @@ describe("runDiscover onResult hook (#6522)", () => { }); describe("eligibility-exclusion signal tracking (#7982)", () => { + // #8544: `captured` keeps the full event (metadata included); `fired` stays the pre-#8544 projection so + // every existing assertion in this block is untouched. + const captured: Array> = []; function fakeSignalStore() { const fired: Array<{ ruleId: string; targetKey: string; outcome: string }> = []; + captured.length = 0; return { fired, + captured, store: { recordRuleFired: vi.fn(async (event: { ruleId: string; targetKey: string; outcome: string }) => { + captured.push({ ...event }); fired.push({ ruleId: event.ruleId, targetKey: event.targetKey, outcome: event.outcome }); }), recordHumanOverride: vi.fn(async () => undefined), @@ -1674,6 +1680,82 @@ describe("runDiscover onResult hook (#6522)", () => { ]); }); + // #8544: bounded candidate-context metadata on the same fired events. + describe("bounded candidate context (#8544)", () => { + const runExcluded = async (issueOverrides: Record) => { + const issues = [fanOutIssue({ issueNumber: 2, ...issueOverrides })]; + const { opts } = discoverWith(issues, new Map([["acme/widgets", trustworthyProfile]])); + const { captured, store } = fakeSignalStore(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + await runDiscover(["acme/widgets", "--json"], { ...opts, initSignalTrackingStore: () => store }); + return captured[0] as { metadata?: Record } | undefined; + }; + + it("captures labels, assignees and owner when all three are present", async () => { + const event = await runExcluded({ labels: ["blocked"], assignees: ["octocat"], owner: "acme" }); + expect(event?.metadata).toEqual({ labels: ["blocked"], assignees: ["octocat"], owner: "acme" }); + expect(event?.metadata).not.toHaveProperty("truncated"); + }); + + it("omits each field that is absent from the candidate rather than storing null or []", async () => { + const event = await runExcluded({ labels: ["blocked"], assignees: undefined, owner: undefined }); + expect(event?.metadata).toEqual({ labels: ["blocked"] }); + expect(event?.metadata).not.toHaveProperty("assignees"); + expect(event?.metadata).not.toHaveProperty("owner"); + }); + + it("keeps an empty array distinguishable from an absent field", async () => { + const event = await runExcluded({ labels: ["blocked"], assignees: [] }); + expect((event?.metadata as { assignees?: string[] })?.assignees).toEqual([]); + }); + + it("labels exactly at the 50 cap are kept whole with no truncated flag", async () => { + const labels = ["blocked", ...Array.from({ length: 49 }, (_, i) => `l${i}`)]; + const event = await runExcluded({ labels }); + expect((event?.metadata as { labels: string[] }).labels).toHaveLength(50); + expect(event?.metadata).not.toHaveProperty("truncated"); + }); + + it("labels one over the cap are clamped to 50 and flagged truncated", async () => { + const labels = ["blocked", ...Array.from({ length: 50 }, (_, i) => `l${i}`)]; + const event = await runExcluded({ labels }); + expect((event?.metadata as { labels: string[] }).labels).toHaveLength(50); + expect(event?.metadata).toHaveProperty("truncated", true); + }); + + it("assignees exactly at the 25 cap are kept whole; one over is clamped and flagged", async () => { + const atCap = await runExcluded({ labels: ["blocked"], assignees: Array.from({ length: 25 }, (_, i) => `u${i}`) }); + expect((atCap?.metadata as { assignees: string[] }).assignees).toHaveLength(25); + expect(atCap?.metadata).not.toHaveProperty("truncated"); + + const overCap = await runExcluded({ labels: ["blocked"], assignees: Array.from({ length: 26 }, (_, i) => `u${i}`) }); + expect((overCap?.metadata as { assignees: string[] }).assignees).toHaveLength(25); + expect(overCap?.metadata).toHaveProperty("truncated", true); + }); + + it("a string exactly at the 200-char cap is kept whole; one char over is clamped and flagged", async () => { + const atCap = await runExcluded({ labels: ["blocked", "x".repeat(200)] }); + expect((atCap?.metadata as { labels: string[] }).labels[1]).toHaveLength(200); + expect(atCap?.metadata).not.toHaveProperty("truncated"); + + const overCap = await runExcluded({ labels: ["blocked", "x".repeat(201)] }); + expect((overCap?.metadata as { labels: string[] }).labels[1]).toHaveLength(200); + expect(overCap?.metadata).toHaveProperty("truncated", true); + }); + + it("omits metadata entirely when the candidate carries no context at all (pre-#8544 event shape)", async () => { + const event = await runExcluded({ labels: undefined, assignees: undefined, owner: undefined }); + expect(event).toBeDefined(); + expect(event).not.toHaveProperty("metadata"); + }); + + it("clamps an over-long owner string and flags it", async () => { + const event = await runExcluded({ labels: ["blocked"], owner: "o".repeat(201) }); + expect((event?.metadata as { owner: string }).owner).toHaveLength(200); + expect(event?.metadata).toHaveProperty("truncated", true); + }); + }); + it("records nothing when nothing was excluded", async () => { const issues = [fanOutIssue({ issueNumber: 1, labels: ["help wanted"] })]; const { opts } = discoverWith(issues, new Map([["acme/widgets", trustworthyProfile]]));