diff --git a/src/signals/improvement.ts b/src/signals/improvement.ts new file mode 100644 index 0000000000..9c00dae7f9 --- /dev/null +++ b/src/signals/improvement.ts @@ -0,0 +1,259 @@ +// Deterministic PR-improvement signal (#4742, sub-issue E of epic #4737): the positive-axis counterpart to +// src/signals/slop.ts's risk-only score. Where slop.ts asks "does this diff look low-effort or risky", this +// module asks "does this diff show measurable structural improvement" — reduced complexity, resolved +// duplication, higher patch coverage, added test evidence. Deterministic tier ONLY: no LLM call lives here. +// The LLM-tier judgment (`ModelReview.valueAssessment`, src/services/ai-review.ts, #4743/#4754) is a +// genuinely separate axis combined with this score at the SURFACING layer (a later sub-issue, #4744) — +// never blended into `improvementScore` itself. +// +// Activation wiring already exists (#4738/#4753: `isImprovementSignalEnabled` + the `improvementSignal` +// ConvergedFeatureKey) but nothing calls `resolveConvergedFeature` for it yet, and this module is not an +// exception — it is a pure, standalone computation consumed only by its own tests until the panel-surfacing +// sub-issue (#4744) wires a caller. It carries NO gate/blocker power (epic design constraint 2): unlike +// slop.ts's header comment ("the ONLY thing allowed to gate"), `improvementScore` must never appear in +// evaluateGateCheck or any blocker path. +// +// Two of the four inputs (complexityDeltas/duplicationDeltas) are REES (review-enrichment service) +// findings. REES is a separate deployable (its own package.json/tsconfig, not a root workspace member — see +// review-enrichment/), so its types are not directly importable here; the shapes below are a structural +// mirror of REES's ComplexityDeltaFinding/DuplicationDeltaFinding (review-enrichment/src/types.ts). As of +// this PR, no channel threads REES's structured `findings` (as opposed to its rendered prompt text) into the +// main app at all — src/review/enrichment-wire.ts only splices REES's pre-rendered { promptSection, +// systemSuffix } into the AI review prompt and never parses `brief.findings`. Likewise, no part of this +// codebase's signal pipeline currently extracts a structured number from Codecov's codecov/patch check (only +// its human-readable text summary reaches src/review/grounding-wire.ts / src/review/unified-comment.ts, e.g. +// "60% of diff hit (target 97%)", for display, not computation). So today, callers of this module have no +// live source for complexityDeltas, duplicationDeltas, or patchCoverageDeltaPercent — all three are honest +// gaps, not yet wired by design (a later sub-issue's job), and this module must degrade cleanly when they're +// absent (see "insufficient signal" below) rather than fabricate a neutral score. +import { buildMissingTestEvidenceFinding, type SlopChangedFile } from "./slop"; +import { isCodeFile } from "./path-matchers"; +import type { SignalFinding } from "./engine"; + +export type ImprovementBand = "insufficient-signal" | "none" | "minor" | "moderate" | "significant"; + +/** Structural mirror of REES's `ComplexityDeltaFinding` (review-enrichment/src/types.ts, #4740) — see the + * module comment for why this isn't imported directly. A negative `delta` is an improvement (the function + * got simpler); a positive `delta` is a regression — both signs can appear in the same array, since REES + * reports every function whose body changed, not just the ones that improved. */ +export type ComplexityDeltaLike = { + file: string; + line: number; + name: string; + before: number; + after: number; + delta: number; +}; + +/** Structural mirror of REES's `DuplicationDeltaFinding` (review-enrichment/src/types.ts, #4741) — see the + * module comment for why this isn't imported directly. Every entry already represents a RESOLVED duplicate + * pair by construction (REES only emits this finding for a pair present pre-PR and no longer both present + * after), so array presence alone — no sign or threshold check — is the positive signal. */ +export type DuplicationDeltaLike = { + file: string; + line: number; + duplicateOfLine: number; + lines: number; +}; + +export type StructuralImprovementInput = { + /** REES complexity-delta analyzer findings for this PR (#4740). Undefined/empty ⇒ the complexity axis has + * nothing to measure for this PR (contributes to "insufficient signal", not to a `none` verdict). */ + complexityDeltas?: ComplexityDeltaLike[] | undefined; + /** REES duplication-delta analyzer findings for this PR (#4741). Undefined/empty ⇒ the duplication axis + * has nothing to measure for this PR. */ + duplicationDeltas?: DuplicationDeltaLike[] | undefined; + /** (after - before) patch/diff coverage percentage for this PR, reusing Codecov's own `codecov/patch` + * number rather than recomputing it — no caller wires a live figure yet (see the module comment). + * Undefined ⇒ the coverage axis has nothing to measure for this PR. */ + patchCoverageDeltaPercent?: number | undefined; + /** Same changed-file/test-evidence inputs slop.ts's own `missingTestEvidence` signal reads, reused + * verbatim (not re-derived) so both signals agree on what counts as test evidence. */ + changedFiles?: SlopChangedFile[] | undefined; + tests?: string[] | undefined; + testFiles?: string[] | undefined; +}; + +export type StructuralImprovementAssessment = { + improvementScore: number; + band: ImprovementBand; + findings: SignalFinding[]; +}; + +// The two REES structural-delta analyzers (complexity/duplication) are the epic's namesake "structural" +// signals and weigh 35 each — either ALONE reaches `moderate` (31-59) and any two reach `significant` +// (60-100). Coverage-delta and test-evidence are corroborating (weigh 20/10): real signals, but each is a +// proxy for improvement rather than a directly-observed structural change, so neither alone should out-rank +// a single structural signal, and both together (30) still sit below a single structural signal (35). +// `clamp(.,0,100)` keeps the stacked score bounded even though the current weights already sum to exactly 100. +export const IMPROVEMENT_WEIGHTS = { + reducedComplexity: 35, + resolvedDuplication: 35, + increasedPatchCoverage: 20, + addedTestEvidence: 10, +} as const; + +export const IMPROVEMENT_RUBRIC_MARKDOWN = [ + "# Gittensory structural-improvement rubric", + "", + "- `insufficient-signal`: none of the four inputs had anything to measure", + "- `none`: 0", + "- `minor`: 1-30", + "- `moderate`: 31-59", + "- `significant`: 60-100", + "", + "Current deterministic signals:", + "- reduced cyclomatic complexity in an existing function (before/after delta)", + "- resolved duplication (a pre-PR duplicate pair no longer both present)", + "- increased patch/diff coverage (Codecov codecov/patch before/after)", + "- added test evidence alongside a code change", +].join("\n"); + +export function buildStructuralImprovementAssessment(input: StructuralImprovementInput): StructuralImprovementAssessment { + const findings: SignalFinding[] = []; + const reducedComplexityFinding = buildReducedComplexityFinding(input); + const resolvedDuplicationFinding = buildResolvedDuplicationFinding(input); + const increasedPatchCoverageFinding = buildIncreasedPatchCoverageFinding(input); + const addedTestEvidenceFinding = buildAddedTestEvidenceFinding(input); + if (reducedComplexityFinding) findings.push(reducedComplexityFinding); + if (resolvedDuplicationFinding) findings.push(resolvedDuplicationFinding); + if (increasedPatchCoverageFinding) findings.push(increasedPatchCoverageFinding); + if (addedTestEvidenceFinding) findings.push(addedTestEvidenceFinding); + + const improvementScore = clamp( + (reducedComplexityFinding ? IMPROVEMENT_WEIGHTS.reducedComplexity : 0) + + (resolvedDuplicationFinding ? IMPROVEMENT_WEIGHTS.resolvedDuplication : 0) + + (increasedPatchCoverageFinding ? IMPROVEMENT_WEIGHTS.increasedPatchCoverage : 0) + + (addedTestEvidenceFinding ? IMPROVEMENT_WEIGHTS.addedTestEvidence : 0), + 0, + 100, + ); + + return { + improvementScore, + band: improvementBandFor(improvementScore, hasApplicableSignal(input)), + findings, + }; +} + +// True when at least one of the four axes had ANYTHING to measure for this PR, regardless of whether that +// axis showed improvement — distinguishes a genuine `none` verdict (measured, found no improvement) from +// `insufficient-signal` (nothing measurable at all, e.g. a docs-only PR with nothing for the complexity/ +// duplication analyzers to look at, no coverage figure, and no code files to check for test evidence). +function hasApplicableSignal(input: StructuralImprovementInput): boolean { + return ( + hasEntries(input.complexityDeltas) || + hasEntries(input.duplicationDeltas) || + finitePatchCoverageDelta(input.patchCoverageDeltaPercent) !== undefined || + hasCodeFileToEvaluate(input.changedFiles) + ); +} + +function hasEntries(list: T[] | undefined): boolean { + return (list?.length ?? 0) > 0; +} + +// Guards against a non-finite caller-supplied figure (NaN/±Infinity) so it is treated identically to +// "no figure supplied" everywhere it is read, rather than silently producing a nonsensical finding or an +// inconsistency between hasApplicableSignal and buildIncreasedPatchCoverageFinding. +function finitePatchCoverageDelta(value: number | undefined): number | undefined { + return typeof value === "number" && Number.isFinite(value) ? value : undefined; +} + +function hasCodeFileToEvaluate(changedFiles: SlopChangedFile[] | undefined): boolean { + return (changedFiles ?? []).some((file) => Boolean(file.path) && isCodeFile(file.path)); +} + +// Fires when at least one function's complexity genuinely dropped (a negative delta) after this PR. Mixed +// signs are expected in the SAME array (REES reports every function whose body changed, not just the ones +// that improved), so this counts strictly `delta < 0` entries rather than trusting array presence alone — +// unlike resolvedDuplication, where presence alone is already the positive fact (see DuplicationDeltaLike). +export function buildReducedComplexityFinding(input: StructuralImprovementInput): SignalFinding | null { + const deltas = input.complexityDeltas ?? []; + if (deltas.length === 0) return null; + const improvedCount = deltas.filter((finding) => finding.delta < 0).length; + if (improvedCount === 0) return null; + // Only an integer count is interpolated, so the text is public-safe by construction (mirrors slop.ts). + const detail = `${improvedCount} function(s) have lower cyclomatic complexity after this pull request.`; + return { + code: "reduced_complexity", + title: "Complexity went down", + severity: "info", + detail, + action: "No action needed — this is a positive signal.", + publicText: detail, + }; +} + +// Every DuplicationDeltaLike entry already IS a resolved pair by construction (see the type's doc comment), +// so array presence alone — no sign or threshold check — is the positive fact. +export function buildResolvedDuplicationFinding(input: StructuralImprovementInput): SignalFinding | null { + const deltas = input.duplicationDeltas ?? []; + if (deltas.length === 0) return null; + const detail = `${deltas.length} previously-duplicated code block(s) were consolidated or removed by this pull request.`; + return { + code: "resolved_duplication", + title: "Duplication went down", + severity: "info", + detail, + action: "No action needed — this is a positive signal.", + publicText: detail, + }; +} + +// Fires only when the caller-supplied figure is a genuine, finite increase (> 0) — an absent, zero, or +// negative figure never fires. The number is interpolated verbatim (never file/diff content), so this stays +// public-safe; rounding/precision is the caller's responsibility. +export function buildIncreasedPatchCoverageFinding(input: StructuralImprovementInput): SignalFinding | null { + const delta = finitePatchCoverageDelta(input.patchCoverageDeltaPercent); + if (delta === undefined || delta <= 0) return null; + const detail = `Patch coverage increased by ${delta} percentage point(s) compared to the base branch.`; + return { + code: "increased_patch_coverage", + title: "Patch coverage went up", + severity: "info", + detail, + action: "No action needed — this is a positive signal.", + publicText: detail, + }; +} + +// Reuses slop.ts's own missingTestEvidence computation (rather than re-deriving isCodeFile/isTestFile +// heuristics here) so the two signals can never disagree about what counts as test evidence. A null result +// from that function is ambiguous by itself (it also returns null when there is no code to test at all), so +// this only treats it as a POSITIVE finding when there was in fact a code file to evaluate. +export function buildAddedTestEvidenceFinding(input: StructuralImprovementInput): SignalFinding | null { + if (!hasCodeFileToEvaluate(input.changedFiles)) return null; + const missingTestEvidence = buildMissingTestEvidenceFinding({ + changedFiles: input.changedFiles, + tests: input.tests, + testFiles: input.testFiles, + }); + if (missingTestEvidence) return null; + const detail = "Code changes are accompanied by test evidence."; + return { + code: "added_test_evidence", + title: "Change carries test evidence", + severity: "info", + detail, + action: "No action needed — this is a positive signal.", + publicText: detail, + }; +} + +// Bands mirror slop.ts's slopBandFor shape (clean/low/elevated/high), renamed for the positive axis and +// extended with a fifth value: `insufficient-signal` fires whenever NONE of the four inputs had anything to +// measure, so a docs-only PR is never misread as "measured, found no improvement" — a raw score of 0 alone +// cannot distinguish those two cases (see hasApplicableSignal), which is exactly why the band is a separate, +// explicit axis rather than a percentage presented as fact. +function improvementBandFor(improvementScore: number, hasSignal: boolean): ImprovementBand { + if (!hasSignal) return "insufficient-signal"; + if (improvementScore <= 0) return "none"; + if (improvementScore < 31) return "minor"; + if (improvementScore < 60) return "moderate"; + return "significant"; +} + +function clamp(value: number, min: number, max: number): number { + return Math.min(max, Math.max(min, value)); +} diff --git a/test/unit/improvement.test.ts b/test/unit/improvement.test.ts new file mode 100644 index 0000000000..9355e2d80d --- /dev/null +++ b/test/unit/improvement.test.ts @@ -0,0 +1,443 @@ +import { describe, expect, it } from "vitest"; +import { + buildAddedTestEvidenceFinding, + buildIncreasedPatchCoverageFinding, + buildReducedComplexityFinding, + buildResolvedDuplicationFinding, + buildStructuralImprovementAssessment, + IMPROVEMENT_RUBRIC_MARKDOWN, + IMPROVEMENT_WEIGHTS, + type ImprovementBand, + type StructuralImprovementInput, +} from "../../src/signals/improvement"; + +const FORBIDDEN_PUBLIC_TERMS = + /wallet|hotkey|coldkey|mnemonic|reward|payout|raw trust|trust score|scoreability|private reviewability|\/Users|\/home|\/tmp/i; + +describe("buildStructuralImprovementAssessment", () => { + it("exports a rubric describing every band and signal", () => { + expect(IMPROVEMENT_RUBRIC_MARKDOWN).toContain("insufficient-signal"); + expect(IMPROVEMENT_RUBRIC_MARKDOWN).toContain("reduced cyclomatic complexity"); + expect(IMPROVEMENT_RUBRIC_MARKDOWN).toContain("resolved duplication"); + expect(IMPROVEMENT_RUBRIC_MARKDOWN).toContain("increased patch/diff coverage"); + expect(IMPROVEMENT_RUBRIC_MARKDOWN).toContain("added test evidence"); + }); + + it("degrades to insufficient-signal when none of the four inputs produced anything (#4742)", () => { + const result = buildStructuralImprovementAssessment({}); + expect(result).toEqual({ improvementScore: 0, band: "insufficient-signal", findings: [] }); + }); + + it("degrades to insufficient-signal for a docs-only PR — nothing for any of the four inputs to measure", () => { + const result = buildStructuralImprovementAssessment({ + changedFiles: [{ path: "docs/guide.md", additions: 40, deletions: 2 }], + }); + expect(result).toEqual({ improvementScore: 0, band: "insufficient-signal", findings: [] }); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("is NOT insufficient-signal when a code file was there to check for test evidence, even with no findings", () => { + // Applicable (a code file exists to evaluate) but nothing fired -- a genuine `none`, distinct from + // insufficient-signal, even though both share improvementScore === 0. + const result = buildStructuralImprovementAssessment({ + changedFiles: [{ path: "src/widget.ts", additions: 20, deletions: 5 }], + }); + expect(result).toEqual({ improvementScore: 0, band: "none", findings: [] }); + }); + + it("is NOT insufficient-signal when complexity deltas were measured but none improved (all regressions)", () => { + // The complexity axis had something to look at (the array is non-empty) -- this must read as `none`, + // never insufficient-signal, even though the aggregate improvementScore is still 0. + const result = buildStructuralImprovementAssessment({ + complexityDeltas: [{ file: "src/a.ts", line: 12, name: "parse", before: 3, after: 9, delta: 6 }], + }); + expect(result).toEqual({ improvementScore: 0, band: "none", findings: [] }); + }); + + it("is NOT insufficient-signal when a patch-coverage figure is present but zero or negative", () => { + expect(buildStructuralImprovementAssessment({ patchCoverageDeltaPercent: 0 })).toEqual({ + improvementScore: 0, + band: "none", + findings: [], + }); + expect(buildStructuralImprovementAssessment({ patchCoverageDeltaPercent: -3.5 })).toEqual({ + improvementScore: 0, + band: "none", + findings: [], + }); + }); + + it("treats a non-finite patch-coverage figure (NaN/Infinity) as absent, not as a zero/negative applicable figure", () => { + expect(buildStructuralImprovementAssessment({ patchCoverageDeltaPercent: Number.NaN })).toEqual({ + improvementScore: 0, + band: "insufficient-signal", + findings: [], + }); + expect(buildStructuralImprovementAssessment({ patchCoverageDeltaPercent: Number.POSITIVE_INFINITY })).toEqual({ + improvementScore: 0, + band: "insufficient-signal", + findings: [], + }); + }); + + it("reaches `moderate` from a single structural signal (reduced complexity alone)", () => { + const result = buildStructuralImprovementAssessment({ + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.reducedComplexity); + expect(result.band).toBe("moderate"); + expect(result.findings).toEqual([expect.objectContaining({ code: "reduced_complexity", severity: "info" })]); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("reaches `moderate` from a single structural signal (resolved duplication alone)", () => { + const result = buildStructuralImprovementAssessment({ + duplicationDeltas: [{ file: "src/a.ts", line: 10, duplicateOfLine: 40, lines: 12 }], + }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.resolvedDuplication); + expect(result.band).toBe("moderate"); + expect(result.findings).toEqual([expect.objectContaining({ code: "resolved_duplication", severity: "info" })]); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("reaches `minor` from a single corroborating signal (patch coverage alone)", () => { + const result = buildStructuralImprovementAssessment({ patchCoverageDeltaPercent: 5 }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.increasedPatchCoverage); + expect(result.band).toBe("minor"); + expect(result.findings).toEqual([expect.objectContaining({ code: "increased_patch_coverage", severity: "info" })]); + }); + + it("reaches `minor` from a single corroborating signal (test evidence alone)", () => { + const result = buildStructuralImprovementAssessment({ + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.addedTestEvidence); + expect(result.band).toBe("minor"); + expect(result.findings).toEqual([expect.objectContaining({ code: "added_test_evidence", severity: "info" })]); + }); + + it("stacks both corroborating signals (30) to a band that still sits below a single structural signal (35)", () => { + const result = buildStructuralImprovementAssessment({ + patchCoverageDeltaPercent: 5, + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.increasedPatchCoverage + IMPROVEMENT_WEIGHTS.addedTestEvidence); + expect(result.improvementScore).toBe(30); + expect(result.band).toBe("minor"); + expect(result.findings.map((finding) => finding.code).sort()).toEqual(["added_test_evidence", "increased_patch_coverage"]); + }); + + it("reaches `significant` when both structural signals fire together", () => { + const result = buildStructuralImprovementAssessment({ + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }], + }); + expect(result.improvementScore).toBe(IMPROVEMENT_WEIGHTS.reducedComplexity + IMPROVEMENT_WEIGHTS.resolvedDuplication); + expect(result.band).toBe("significant"); + expect(result.findings.map((finding) => finding.code).sort()).toEqual(["reduced_complexity", "resolved_duplication"]); + }); + + it("combines all four inputs into one aggregate score/band with no cross-band leakage", () => { + const result = buildStructuralImprovementAssessment({ + complexityDeltas: [ + { file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }, + { file: "src/a.ts", line: 40, name: "bar", before: 2, after: 6, delta: 4 }, // a regression mixed in + ], + duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }], + patchCoverageDeltaPercent: 5, + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }); + expect(result.improvementScore).toBe(100); + expect(result.band).toBe("significant"); + expect(result.findings.map((finding) => finding.code).sort()).toEqual([ + "added_test_evidence", + "increased_patch_coverage", + "reduced_complexity", + "resolved_duplication", + ]); + // The mixed-sign complexity array still reports only the ONE improved function, proving the finding + // filters by sign rather than trusting array presence (unlike resolvedDuplication). + const complexityFinding = result.findings.find((finding) => finding.code === "reduced_complexity"); + expect(complexityFinding?.detail).toContain("1 function(s)"); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("returns identical output for identical input (determinism)", () => { + const input: StructuralImprovementInput = { + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + patchCoverageDeltaPercent: 2.5, + }; + expect(buildStructuralImprovementAssessment(input)).toEqual(buildStructuralImprovementAssessment(input)); + }); + + describe("golden fixtures & band-boundary determinism", () => { + const goldenFixtures: Array<{ + name: string; + input: StructuralImprovementInput; + improvementScore: number; + band: ImprovementBand; + codes: string[]; + }> = [ + { name: "insufficient-signal -- no metadata at all", input: {}, improvementScore: 0, band: "insufficient-signal", codes: [] }, + { + name: "insufficient-signal -- docs-only PR", + input: { changedFiles: [{ path: "docs/guide.md", additions: 40, deletions: 2 }] }, + improvementScore: 0, + band: "insufficient-signal", + codes: [], + }, + { + name: "none -- code change with no positive signal", + input: { changedFiles: [{ path: "src/widget.ts", additions: 20, deletions: 5 }] }, + improvementScore: 0, + band: "none", + codes: [], + }, + { + name: "minor -- patch coverage increase alone", + input: { patchCoverageDeltaPercent: 5 }, + improvementScore: IMPROVEMENT_WEIGHTS.increasedPatchCoverage, + band: "minor", + codes: ["increased_patch_coverage"], + }, + { + name: "minor -- test evidence alone", + input: { + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }, + improvementScore: IMPROVEMENT_WEIGHTS.addedTestEvidence, + band: "minor", + codes: ["added_test_evidence"], + }, + { + // Boundary case: both corroborating signals stack to exactly 30 -- still `minor` (1-30), not + // `moderate` (31-59), proving the corroborating pair never out-ranks a single structural signal. + name: "minor -- both corroborating signals stack to exactly 30 (boundary)", + input: { + patchCoverageDeltaPercent: 5, + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }, + improvementScore: 30, + band: "minor", + codes: ["added_test_evidence", "increased_patch_coverage"], + }, + { + name: "moderate -- reduced complexity alone", + input: { complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }] }, + improvementScore: IMPROVEMENT_WEIGHTS.reducedComplexity, + band: "moderate", + codes: ["reduced_complexity"], + }, + { + name: "moderate -- resolved duplication alone", + input: { duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }] }, + improvementScore: IMPROVEMENT_WEIGHTS.resolvedDuplication, + band: "moderate", + codes: ["resolved_duplication"], + }, + { + name: "significant -- both structural signals", + input: { + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }], + }, + improvementScore: 70, + band: "significant", + codes: ["reduced_complexity", "resolved_duplication"], + }, + { + name: "significant -- all four signals", + input: { + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }], + patchCoverageDeltaPercent: 5, + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }, + improvementScore: 100, + band: "significant", + codes: ["added_test_evidence", "increased_patch_coverage", "reduced_complexity", "resolved_duplication"], + }, + ]; + + it.each(goldenFixtures)("scores the $name fixture to its documented band", (fixture) => { + const result = buildStructuralImprovementAssessment(fixture.input); + expect(result.improvementScore).toBe(fixture.improvementScore); + expect(result.band).toBe(fixture.band); + expect(result.findings.map((finding) => finding.code).sort()).toEqual(fixture.codes); + expect(JSON.stringify(result)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("returns identical improvementScore and findings for identical metadata (determinism)", () => { + for (const fixture of goldenFixtures) { + expect(buildStructuralImprovementAssessment(fixture.input)).toEqual(buildStructuralImprovementAssessment(fixture.input)); + } + }); + + it("keeps every fixture score within the clamped 0..100 range (invariant)", () => { + for (const fixture of goldenFixtures) { + const { improvementScore } = buildStructuralImprovementAssessment(fixture.input); + expect(improvementScore).toBeGreaterThanOrEqual(0); + expect(improvementScore).toBeLessThanOrEqual(100); + } + }); + + it("never mixes insufficient-signal with a non-zero score, and never mixes `none` with a non-zero score (invariant)", () => { + for (const fixture of goldenFixtures) { + const result = buildStructuralImprovementAssessment(fixture.input); + if (result.band === "insufficient-signal" || result.band === "none") { + expect(result.improvementScore).toBe(0); + expect(result.findings).toEqual([]); + } + } + }); + }); +}); + +describe("buildReducedComplexityFinding", () => { + it("returns null when no complexity deltas are supplied (undefined or empty)", () => { + expect(buildReducedComplexityFinding({})).toBeNull(); + expect(buildReducedComplexityFinding({ complexityDeltas: [] })).toBeNull(); + }); + + it("returns null when every delta is a regression or unchanged (delta >= 0)", () => { + expect( + buildReducedComplexityFinding({ + complexityDeltas: [ + { file: "src/a.ts", line: 1, name: "foo", before: 3, after: 3, delta: 0 }, + { file: "src/a.ts", line: 20, name: "bar", before: 2, after: 5, delta: 3 }, + ], + }), + ).toBeNull(); + }); + + it("fires and counts only the improved (delta < 0) entries out of a mixed-sign array", () => { + const finding = buildReducedComplexityFinding({ + complexityDeltas: [ + { file: "src/a.ts", line: 1, name: "foo", before: 12, after: 4, delta: -8 }, + { file: "src/a.ts", line: 20, name: "bar", before: 2, after: 9, delta: 7 }, + { file: "src/b.ts", line: 5, name: "baz", before: 20, after: 11, delta: -9 }, + ], + }); + expect(finding).toMatchObject({ code: "reduced_complexity", severity: "info" }); + expect(finding?.detail).toContain("2 function(s)"); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); +}); + +describe("buildResolvedDuplicationFinding", () => { + it("returns null when no duplication deltas are supplied (undefined or empty)", () => { + expect(buildResolvedDuplicationFinding({})).toBeNull(); + expect(buildResolvedDuplicationFinding({ duplicationDeltas: [] })).toBeNull(); + }); + + it("fires and reports the resolved count when duplication deltas are present", () => { + const finding = buildResolvedDuplicationFinding({ + duplicationDeltas: [ + { file: "src/a.ts", line: 5, duplicateOfLine: 55, lines: 9 }, + { file: "src/b.ts", line: 12, duplicateOfLine: 88, lines: 6 }, + ], + }); + expect(finding).toMatchObject({ code: "resolved_duplication", severity: "info" }); + expect(finding?.detail).toContain("2 previously-duplicated"); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); +}); + +describe("buildIncreasedPatchCoverageFinding", () => { + it("returns null when no figure is supplied", () => { + expect(buildIncreasedPatchCoverageFinding({})).toBeNull(); + }); + + it("returns null for a non-finite figure (NaN or Infinity)", () => { + expect(buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: Number.NaN })).toBeNull(); + expect(buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: Number.POSITIVE_INFINITY })).toBeNull(); + expect(buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: Number.NEGATIVE_INFINITY })).toBeNull(); + }); + + it("returns null for a zero or negative figure", () => { + expect(buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: 0 })).toBeNull(); + expect(buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: -12.5 })).toBeNull(); + }); + + it("fires and interpolates the figure for a genuine increase", () => { + const finding = buildIncreasedPatchCoverageFinding({ patchCoverageDeltaPercent: 4.2 }); + expect(finding).toMatchObject({ code: "increased_patch_coverage", severity: "info" }); + expect(finding?.detail).toContain("4.2"); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); +}); + +describe("buildAddedTestEvidenceFinding", () => { + it("returns null when there are no changed files at all", () => { + expect(buildAddedTestEvidenceFinding({})).toBeNull(); + }); + + it("returns null when changed files exist but none are code files (docs-only)", () => { + expect( + buildAddedTestEvidenceFinding({ + changedFiles: [{ path: "docs/guide.md", additions: 40, deletions: 2 }], + }), + ).toBeNull(); + }); + + it("ignores a changed-file entry with an empty path when checking for a code file to evaluate", () => { + expect( + buildAddedTestEvidenceFinding({ + changedFiles: [{ path: "", additions: 40, deletions: 2 }], + }), + ).toBeNull(); + }); + + it("returns null when a code file changed with no accompanying test evidence", () => { + expect( + buildAddedTestEvidenceFinding({ + changedFiles: [{ path: "src/widget.ts", additions: 20, deletions: 5 }], + }), + ).toBeNull(); + }); + + it("fires when a changed test FILE accompanies the code change", () => { + const finding = buildAddedTestEvidenceFinding({ + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }); + expect(finding).toMatchObject({ code: "added_test_evidence", severity: "info" }); + expect(JSON.stringify(finding)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + }); + + it("fires when external testFiles evidence is supplied instead of a changed test file", () => { + const finding = buildAddedTestEvidenceFinding({ + changedFiles: [{ path: "src/registry/sync.ts", additions: 12, deletions: 0 }], + testFiles: ["internal/cache_test.go"], + }); + expect(finding).toMatchObject({ code: "added_test_evidence" }); + }); + + it("fires when external tests evidence (test identifiers) is supplied", () => { + const finding = buildAddedTestEvidenceFinding({ + changedFiles: [{ path: "src/registry/sync.ts", additions: 12, deletions: 0 }], + tests: ["sync_test.go::TestRetryBackoff"], + }); + expect(finding).toMatchObject({ code: "added_test_evidence" }); + }); +});