From 6cc59dc5a4929563ff4e151e03dd9c3c0d37d644 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:24:15 -0700 Subject: [PATCH] feat(signals): add deterministic structural-improvement assessment (#4742) Adds src/signals/improvement.ts, the positive-axis counterpart to slop.ts's risk-only score (sub-issue E of epic #4737): a buildStructuralImprovementAssessment entrypoint that combines four deterministic inputs -- REES's complexity-delta and duplication-delta findings (#4740/#4741), a patch-coverage-delta figure reusing Codecov's own codecov/patch numbers, and a test-evidence signal reusing slop.ts's own buildMissingTestEvidenceFinding -- into one { improvementScore, band, findings } result, mirroring SlopAssessment's shape deliberately. Deterministic tier only, matching the epic's tier split: no LLM call lives here, and the LLM-tier judgment (ModelReview.valueAssessment, already shipped) is combined at the surfacing layer in a later sub-issue, never blended into this score. This module carries no gate/blocker power -- it is a pure, standalone computation consumed only by its own tests, the same activation-wiring-only pattern the epic's foundation sub-issue already used. Bands are none/minor/moderate/significant, plus a distinct insufficient-signal band for a PR where none of the four inputs had anything to measure (e.g. docs-only) -- so an analyzer's silence is never misread as "measured, found no improvement" (score 0 alone cannot distinguish those two cases). Two of the four inputs are honest gaps rather than live data: REES is a separate deployable (not a workspace member, not in the root tsconfig), so the complexity/duplication-delta types are structurally mirrored here rather than imported, and nothing in this codebase currently threads REES's structured findings (as opposed to its rendered prompt text) into the main app, or extracts a structured number from Codecov's check-run text. Wiring a live source for both is follow-up work, not part of this change. --- src/signals/improvement.ts | 259 ++++++++++++++++++++ test/unit/improvement.test.ts | 443 ++++++++++++++++++++++++++++++++++ 2 files changed, 702 insertions(+) create mode 100644 src/signals/improvement.ts create mode 100644 test/unit/improvement.test.ts 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" }); + }); +});