diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6685ecd5f..60176ec5a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -146,9 +146,11 @@ jobs: override_branch: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.ref || github.ref_name }} override_commit: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} override_pr: ${{ github.event_name == 'pull_request' && github.event.pull_request.number || '' }} - # Coverage upload is part of the hard gate: upload or service errors - # should fail CI instead of allowing a PR to merge without patch data. - fail_ci_if_error: true + # Coverage upload is part of the hard gate for trusted CI (main pushes and + # upstream-repo PRs with CODECOV_TOKEN). Fork PRs cannot read repo secrets, + # so Codecov rejects tokenless uploads to protected branches — keep those + # uploads best-effort so validate-code still runs tests and emits lcov. + fail_ci_if_error: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork != true }} # Test results are useful Codecov annotations, not the coverage gate. Keep # their upload non-blocking so a JUnit ingestion hiccup does not fail CI # after the tests and hard coverage upload have already passed. diff --git a/src/services/score-breakdown.ts b/src/services/score-breakdown.ts index 1d20e5873..766d76f01 100644 --- a/src/services/score-breakdown.ts +++ b/src/services/score-breakdown.ts @@ -131,6 +131,63 @@ function mergedHistoryBreakdown(preview: ScorePreviewResult): ScoreMultiplierBre }; } +// Sibling of mergedHistoryBreakdown for the issue-discovery validity floor (upstream +// MIN_VALID_SOLVED_ISSUES and MIN_ISSUE_CREDIBILITY). Mirrors preview.ts: the multiplier +// stays 1 unless linked-issue mode is active AND both history fields are observed. +function issueDiscoveryHistoryBreakdown(preview: ScorePreviewResult): ScoreMultiplierBreakdown { + const { issueDiscoveryHistoryMultiplier } = preview.scoreEstimate; + const { validSolvedIssues, validSolvedIssuesFloor, issueCredibility, issueCredibilityFloor } = preview.gates; + const linkedIssueMode = preview.linkedIssueMultiplier.mode; + + if (linkedIssueMode === "none") { + return { + component: "issueDiscoveryHistoryMultiplier", + band: "neutral", + summary: + "Issue-discovery validity floor is not enforced for this preview (linked-issue mode is inactive; upstream only gates previews that claim linked-issue scoring).", + lever: "Use a linked-issue preview when planning issue-discovery work so validity floors can be evaluated.", + leverageScore: 0, + }; + } + + const hasValidSolved = validSolvedIssues !== undefined; + const hasIssueCredibility = issueCredibility !== undefined; + if (!hasValidSolved && !hasIssueCredibility) { + return { + component: "issueDiscoveryHistoryMultiplier", + band: "neutral", + summary: `Issue-discovery validity floor is not enforced for this preview (contributor issue-history is unobserved; upstream floors are ${validSolvedIssuesFloor} valid solved and ${issueCredibilityFloor} credibility).`, + lever: "No action needed for this preview; the validity floor applies once both valid solved count and issue credibility are observed.", + leverageScore: 0, + }; + } + if (!hasValidSolved || !hasIssueCredibility) { + return { + component: "issueDiscoveryHistoryMultiplier", + band: "neutral", + summary: hasValidSolved + ? "Issue-discovery validity floor is not enforced for this preview (valid solved count is observed but issue credibility is not; upstream requires both before gating)." + : "Issue-discovery validity floor is not enforced for this preview (issue credibility is observed but valid solved count is not; upstream requires both before gating).", + lever: "Supply both valid solved-issue count and issue credibility before relying on issue-discovery validity floors.", + leverageScore: 0, + }; + } + + const band = bandForMultiplier(issueDiscoveryHistoryMultiplier); + const meetsFloor = validSolvedIssues >= validSolvedIssuesFloor && issueCredibility >= issueCredibilityFloor; + return { + component: "issueDiscoveryHistoryMultiplier", + band, + summary: meetsFloor + ? `Issue-discovery history (${validSolvedIssues} valid solved, credibility ${roundBand(issueCredibility)}) meets upstream floors (${validSolvedIssuesFloor} valid solved, ${issueCredibilityFloor} credibility).` + : `Issue-discovery history (${validSolvedIssues} valid solved, credibility ${roundBand(issueCredibility)}) is below upstream floors (${validSolvedIssuesFloor} valid solved, ${issueCredibilityFloor} credibility), so this preview is zeroed.`, + lever: meetsFloor + ? "Keep building valid solved-issue history with strong issue credibility." + : "Close more valid solved issues and improve issue credibility before relying on issue-discovery scoring.", + leverageScore: meetsFloor ? 8 : 100, + }; +} + // Upstream time-decay (#703), env-gated by SCORING_TIME_DECAY_ENABLED (default OFF) and opted into per-preview // via input.applyTimeDecay. When the flag is off (the common case) or the PR is fresh, the multiplier is 1 and // the breakdown reads as "not enabled" / "fresh" — surfacing the value is a no-op for those previews but @@ -348,6 +405,7 @@ export function explainScoreBreakdown(preview: ScorePreviewResult): ScoreBreakdo openPrBreakdown(preview), openIssueBreakdown(preview), mergedHistoryBreakdown(preview), + issueDiscoveryHistoryBreakdown(preview), timeDecayBreakdown(preview), nonCodeCapBreakdown(preview), ].map((entry) => ({ diff --git a/test/unit/codecov-policy.test.ts b/test/unit/codecov-policy.test.ts index 9d89fb8b7..124e243c3 100644 --- a/test/unit/codecov-policy.test.ts +++ b/test/unit/codecov-policy.test.ts @@ -59,7 +59,8 @@ describe("Codecov policy", () => { const coverageUploadWith = record(coverageUpload.with, "coverage upload with"); expect(coverageUploadWith.files).toBe("./coverage/lcov.info"); expect(coverageUploadWith.disable_search).toBe(true); - expect(coverageUploadWith.fail_ci_if_error).toBe(true); + expect(String(coverageUploadWith.fail_ci_if_error)).toContain("fork"); + expect(String(coverageUploadWith.fail_ci_if_error)).toContain("pull_request"); const testResultsUploadWith = record(testResultsUpload.with, "test results upload with"); expect(testResultsUploadWith.report_type).toBe("test_results"); diff --git a/test/unit/github-commands.test.ts b/test/unit/github-commands.test.ts index 4ef09ac10..e70c8e601 100644 --- a/test/unit/github-commands.test.ts +++ b/test/unit/github-commands.test.ts @@ -172,6 +172,16 @@ describe("GitHub mention commands", () => { expect(sanitizePublicComment("Issue-discovery history (2 valid solved, credibility 0.42) is below upstream floors (3 valid solved, 0.8 credibility).")).not.toMatch( /issue-discovery history|valid solved|upstream floors|0\.42|0\.8|\b2\b|\b3\b/i, ); + expect( + sanitizePublicComment( + "Issue-discovery validity floor is not enforced for this preview (valid solved count is observed but issue credibility is not; upstream requires both before gating).", + ), + ).toMatch(/issue private context is not/i); + expect( + sanitizePublicComment( + "Issue-discovery validity floor is not enforced for this preview (issue credibility is observed but valid solved count is not; upstream requires both before gating).", + ), + ).toMatch(/issue private context is observed/i); expect(sanitizePublicComment("Credibility 0.12 is below floor 0.4.")).not.toMatch(/credibility|0\.12|floor|0\.4/i); expect(sanitizePublicComment("open_pr_pressure closed_pr_credibility low_credibility credibility updates")).not.toMatch(/open_pr_pressure|closed_pr_credibility|low_credibility|credibility/i); expect(sanitizePublicComment("Command: @gittensory reviewability")).toContain("@gittensory reviewability"); diff --git a/test/unit/score-breakdown.test.ts b/test/unit/score-breakdown.test.ts index 3b13c7ab5..5e385b5d8 100644 --- a/test/unit/score-breakdown.test.ts +++ b/test/unit/score-breakdown.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { sanitizePublicComment } from "../../src/github/commands"; import { buildScorePreview } from "../../src/scoring/preview"; import { explainScoreBreakdown } from "../../src/services/score-breakdown"; import type { RepositoryRecord, ScoringModelSnapshotRecord } from "../../src/types"; @@ -85,6 +86,7 @@ describe("explainScoreBreakdown", () => { "openPrMultiplier", "openIssueMultiplier", "mergedHistoryMultiplier", + "issueDiscoveryHistoryMultiplier", "timeDecayMultiplier", "nonCodeLineCap", ]), @@ -150,6 +152,120 @@ describe("explainScoreBreakdown", () => { expect(JSON.stringify(over)).not.toMatch(FORBIDDEN); }); + it("explains the issue-discovery validity floor as neutral (unobserved), full (meets floor), and blocked (below floor)", () => { + const issueDiscoveryRepo: RepositoryRecord = { + ...repo, + registryConfig: { ...repo.registryConfig!, issueDiscoveryShare: 0.25 }, + }; + const baseInput = { + repoFullName: issueDiscoveryRepo.fullName, + contributorLogin: "miner", + sourceTokenScore: 40, + totalTokenScore: 60, + sourceLines: 80, + openPrCount: 0, + credibility: 1, + mergedPullRequests: 5, + linkedIssueMode: "standard" as const, + }; + + const unobserved = explainScoreBreakdown(buildScorePreview({ repo: issueDiscoveryRepo, snapshot, input: baseInput })); + expect(unobserved.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")).toMatchObject({ + band: "neutral", + leverageScore: 0, + }); + + const meetsPreview = buildScorePreview({ + repo: issueDiscoveryRepo, + snapshot, + input: { ...baseInput, validSolvedIssues: 4, issueCredibility: 0.9 }, + }); + const meets = explainScoreBreakdown(meetsPreview); + const meetsEntry = meets.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")!; + const meetsRawSummary = `Issue-discovery history (4 valid solved, credibility 0.9) meets upstream floors (${meetsPreview.gates.validSolvedIssuesFloor} valid solved, ${meetsPreview.gates.issueCredibilityFloor} credibility).`; + expect(meetsEntry).toMatchObject({ band: "full" }); + // explainScoreBreakdown sanitizes raw multiplier copy before returning it publicly. + expect(meetsEntry.summary).toBe(sanitizePublicComment(meetsRawSummary)); + + const blockedPreview = buildScorePreview({ + repo: issueDiscoveryRepo, + snapshot, + input: { ...baseInput, validSolvedIssues: 1, issueCredibility: 0.5 }, + }); + const blocked = explainScoreBreakdown(blockedPreview); + const blockedEntry = blocked.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")!; + const blockedRawSummary = `Issue-discovery history (1 valid solved, credibility 0.5) is below upstream floors (${blockedPreview.gates.validSolvedIssuesFloor} valid solved, ${blockedPreview.gates.issueCredibilityFloor} credibility), so this preview is zeroed.`; + expect(blockedEntry).toMatchObject({ + band: "blocked", + leverageScore: 100, + }); + expect(blockedEntry.summary).toBe(sanitizePublicComment(blockedRawSummary)); + expect(blocked.highestLeverageLever.lever).toMatch(/valid solved issues|issue private context/i); + expect(JSON.stringify(blocked)).not.toMatch(FORBIDDEN); + }); + + it("keeps issue-discovery validity neutral when linked-issue mode is inactive even if history is supplied", () => { + const issueDiscoveryRepo: RepositoryRecord = { + ...repo, + registryConfig: { ...repo.registryConfig!, issueDiscoveryShare: 0.25 }, + }; + const preview = buildScorePreview({ + repo: issueDiscoveryRepo, + snapshot, + input: { + repoFullName: issueDiscoveryRepo.fullName, + sourceTokenScore: 40, + totalTokenScore: 60, + sourceLines: 80, + openPrCount: 0, + credibility: 1, + linkedIssueMode: "none", + validSolvedIssues: 4, + issueCredibility: 0.95, + }, + }); + expect(preview.scoreEstimate.issueDiscoveryHistoryMultiplier).toBe(1); + const breakdown = explainScoreBreakdown(preview); + expect(breakdown.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")).toMatchObject({ + band: "neutral", + summary: expect.stringMatching(/linked-issue mode is inactive/i), + leverageScore: 0, + }); + }); + + it("keeps issue-discovery validity neutral when only one history field is observed", () => { + const issueDiscoveryRepo: RepositoryRecord = { + ...repo, + registryConfig: { ...repo.registryConfig!, issueDiscoveryShare: 0.25 }, + }; + const baseInput = { + repoFullName: issueDiscoveryRepo.fullName, + sourceTokenScore: 40, + totalTokenScore: 60, + sourceLines: 80, + openPrCount: 0, + credibility: 1, + mergedPullRequests: 5, + linkedIssueMode: "standard" as const, + }; + + const validSolvedOnlyPreview = buildScorePreview({ repo: issueDiscoveryRepo, snapshot, input: { ...baseInput, validSolvedIssues: 4 } }); + const validSolvedOnly = explainScoreBreakdown(validSolvedOnlyPreview); + const validSolvedOnlyEntry = validSolvedOnly.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")!; + const validSolvedOnlyRawSummary = + "Issue-discovery validity floor is not enforced for this preview (valid solved count is observed but issue credibility is not; upstream requires both before gating)."; + expect(validSolvedOnlyEntry).toMatchObject({ band: "neutral" }); + expect(validSolvedOnlyEntry.summary).toBe(sanitizePublicComment(validSolvedOnlyRawSummary)); + + const credibilityOnlyPreview = buildScorePreview({ repo: issueDiscoveryRepo, snapshot, input: { ...baseInput, issueCredibility: 0.9 } }); + const credibilityOnly = explainScoreBreakdown(credibilityOnlyPreview); + const credibilityOnlyEntry = credibilityOnly.components.find((entry) => entry.component === "issueDiscoveryHistoryMultiplier")!; + const credibilityOnlyRawSummary = + "Issue-discovery validity floor is not enforced for this preview (issue credibility is observed but valid solved count is not; upstream requires both before gating)."; + expect(credibilityOnlyEntry).toMatchObject({ band: "neutral" }); + expect(credibilityOnlyEntry.summary).toBe(sanitizePublicComment(credibilityOnlyRawSummary)); + }); + it("explains an over-threshold open-issue count as a blocked open-issue spam gate", () => { const preview = buildScorePreview({ repo,