Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions apps/gittensory-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -2711,9 +2711,10 @@
"approved",
"blocked",
"stale",
"needs_author",

Check notice on line 2714 in apps/gittensory-ui/public/openapi.json

View check run for this annotation

Loopover ORB / Gittensory Context

Review queue is busy

This repo has a busy review queue in the local Gittensory cache.
"failing_checks",
"missing_tests",
"weak_test_coverage",
"duplicate_prone",
"reviewable",
"should_close_or_withdraw",
Expand Down Expand Up @@ -7620,6 +7621,47 @@
},
"rerunWhen": {
"type": "string"
},
"testCoverage": {
"type": "object",
"properties": {
"classification": {
"type": "string",
"enum": [
"strong",
"adequate",
"weak",
"absent"
]
},
"changedPathCount": {
"type": "number"
},
"sourcePathCount": {
"type": "number"
},
"testPathCount": {
"type": "number"
},
"fixturePathCount": {
"type": "number"
},
"testToChangedRatio": {
"type": "number"
},
"guidance": {
"type": "string"
}
},
"required": [
"classification",
"changedPathCount",
"sourcePathCount",
"testPathCount",
"fixturePathCount",
"testToChangedRatio",
"guidance"
]
}
},
"required": [
Expand All @@ -7628,6 +7670,7 @@
"branch",
"changedFiles",
"testEvidence",
"testCoverage",
"linkedIssues",
"baseFreshness",
"ciStatusHints",
Expand Down
10 changes: 10 additions & 0 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,10 @@
"approved",
"blocked",
"stale",
"needs_author",

Check notice on line 335 in src/openapi/schemas.ts

View check run for this annotation

Loopover ORB / Gittensory Context

Review queue is busy

This repo has a busy review queue in the local Gittensory cache.
"failing_checks",
"missing_tests",
"weak_test_coverage",
"duplicate_prone",
"reviewable",
"should_close_or_withdraw",
Expand Down Expand Up @@ -2158,6 +2159,15 @@
}),
),
}),
testCoverage: z.object({
classification: z.enum(["strong", "adequate", "weak", "absent"]),
changedPathCount: z.number(),
sourcePathCount: z.number(),
testPathCount: z.number(),
fixturePathCount: z.number(),
testToChangedRatio: z.number(),
guidance: z.string(),
}),
linkedIssues: z.array(z.number()),
baseFreshness: z.object({
status: z.enum(["fresh", "stale", "possibly_stale", "unknown"]),
Expand Down
50 changes: 42 additions & 8 deletions src/signals/contributor-open-pr-monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
type PendingPrScenarioDetection,
} from "../scoring/pending-pr-scenarios";
import type { CheckSummaryRecord, PullRequestFileRecord, PullRequestRecord, PullRequestReviewRecord } from "../types";
import { nowIso } from "../utils/json";

Check notice on line 12 in src/signals/contributor-open-pr-monitor.ts

View check run for this annotation

Loopover ORB / Gittensory Context

Review queue is busy

This repo has a busy review queue in the local Gittensory cache.
import { buildRoleContext } from "./engine";
import { isFailingCheckSummary } from "./local-branch";
import { isTestPath } from "./test-evidence";
import { buildTestCoverageSummary, type TestCoverageClassification } from "./test-evidence";

export type OpenPrWorkClassification =
| "approved"
Expand All @@ -21,6 +21,7 @@
| "needs_author"
| "failing_checks"
| "missing_tests"
| "weak_test_coverage"
| "duplicate_prone"
| "reviewable"
| "should_close_or_withdraw"
Expand Down Expand Up @@ -89,7 +90,16 @@
duplicateProne: duplicateNumbers.has(pr.number),
missingTests: missingTestsFromFiles(files),
});
packets.push(buildNextStepPacket(classified, reviews, checks, duplicateNumbers.has(pr.number), missingTestsFromFiles(files)));
packets.push(
buildNextStepPacket(
classified,
reviews,
checks,
duplicateNumbers.has(pr.number),
missingTestsFromFiles(files),
weakTestCoverageFromFiles(files),
),
);
}

const detection = detectPendingPrScenario({
Expand Down Expand Up @@ -128,7 +138,7 @@

export function mapPendingClassToWorkClassification(
classified: ClassifiedOpenPullRequest,
args: { changeRequestCount: number; checkFailureCount: number; duplicateProne: boolean; missingTests: boolean },
args: { changeRequestCount: number; checkFailureCount: number; duplicateProne: boolean; missingTests: boolean; weakTestCoverage: boolean },
): OpenPrWorkClassification {
if (classified.classification === "maintainer_lane") return "maintainer_lane";
if (classified.classification === "draft") return "draft";
Expand All @@ -137,6 +147,7 @@
if (args.checkFailureCount > 0) return "failing_checks";
if (args.changeRequestCount > 0) return "needs_author";
if (args.missingTests) return "missing_tests";
if (args.weakTestCoverage) return "weak_test_coverage";
if (classified.classification === "merge_ready") return "approved";
if (classified.classification === "blocked") return "blocked";
return "reviewable";
Expand All @@ -148,10 +159,17 @@
checks: CheckSummaryRecord[],
duplicateProne: boolean,
missingTests: boolean,
weakTestCoverage: boolean,
): ContributorOpenPrNextStepPacket {
const changeRequestCount = reviews.filter((review) => review.state.toUpperCase() === "CHANGES_REQUESTED").length;
const checkFailureCount = checks.filter(isFailingCheckSummary).length;
const classification = mapPendingClassToWorkClassification(classified, { changeRequestCount, checkFailureCount, duplicateProne, missingTests });
const classification = mapPendingClassToWorkClassification(classified, {
changeRequestCount,
checkFailureCount,
duplicateProne,
missingTests,
weakTestCoverage,
});
const nextSteps = nextStepsForClassification(classification, classified.repoFullName, classified.number);
const summary = `${classified.repoFullName}#${classified.number}: ${classification.replace(/_/g, " ")} — ${classified.title}`;
return {
Expand All @@ -176,6 +194,11 @@
return [`Address review comments on ${ref} and push updates.`, `Reply on the PR thread summarizing what changed.`];
case "missing_tests":
return [`Add or update tests on ${ref} if the repo expects test coverage.`, `Note test commands run in the PR description.`];
case "weak_test_coverage":
return [
`Broaden test or fixture coverage on ${ref} so it matches the source files touched.`,
`Add focused regression tests for the modules changed and note the commands run in the PR description.`,
];
case "duplicate_prone":
return [`Check overlap with other open PRs in ${repoFullName}; close or consolidate duplicates.`, `Comment on ${ref} linking the canonical PR if one exists.`];
case "stale":
Expand Down Expand Up @@ -259,10 +282,18 @@
}

function missingTestsFromFiles(files: PullRequestFileRecord[]): boolean {
if (files.length === 0) return false;
const codeFiles = files.filter((file) => file.path && !isTestPath(file.path));
const testFiles = files.filter((file) => file.path && isTestPath(file.path));
return codeFiles.length > 0 && testFiles.length === 0;
const classification = testCoverageClassificationFromFiles(files);
return classification === "absent";
}

function weakTestCoverageFromFiles(files: PullRequestFileRecord[]): boolean {
return testCoverageClassificationFromFiles(files) === "weak";
}

function testCoverageClassificationFromFiles(files: PullRequestFileRecord[]): TestCoverageClassification | null {
const paths = files.map((file) => file.path).filter(Boolean);
if (paths.length === 0) return null;
return buildTestCoverageSummary(paths).classification;
}

function priorityRank(classification: OpenPrWorkClassification): number {
Expand All @@ -271,6 +302,7 @@
"needs_author",
"duplicate_prone",
"missing_tests",
"weak_test_coverage",
"blocked",
"should_close_or_withdraw",
"stale",
Expand All @@ -294,6 +326,8 @@
buildMonitorGuidance,
duplicatePronePullNumbers,
missingTestsFromFiles,
weakTestCoverageFromFiles,
testCoverageClassificationFromFiles,
priorityRank,
buildNextStepPacket,
};
8 changes: 6 additions & 2 deletions src/signals/local-workspace-intelligence.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { isPassingValidation } from "./local-branch";

Check notice on line 1 in src/signals/local-workspace-intelligence.ts

View check run for this annotation

Loopover ORB / Gittensory Context

Review queue is busy

This repo has a busy review queue in the local Gittensory cache.
import type { LocalBranchAnalysis, LocalBranchAnalysisInput, LocalBranchChangedFile, LocalBranchValidation } from "./local-branch";
import { isTestPath } from "./test-evidence";
import { buildTestCoverageSummary, isTestEvidencePath, type TestCoverageSummary } from "./test-evidence";
import { sanitizeLocalScorerWarnings } from "./local-scorer-diagnostics";

export type LocalWorkspaceIntelligence = {
Expand Down Expand Up @@ -30,6 +30,7 @@
passedValidationCount: number;
commands: LocalBranchValidation[];
};
testCoverage: TestCoverageSummary;
linkedIssues: number[];
baseFreshness: LocalBranchAnalysis["baseFreshness"];
ciStatusHints: string[];
Expand All @@ -55,11 +56,13 @@
changedFiles: LocalBranchChangedFile[];
}): LocalWorkspaceIntelligence {
const validation = args.input.validation ?? [];
const testFileCount = args.changedFiles.filter((file) => isTestPath(file.path)).length;
const changedPaths = args.changedFiles.map((file) => file.path).filter(Boolean);
const testFileCount = changedPaths.filter((path) => isTestEvidencePath(path)).length;
const passedValidationCount = validation.filter(isPassingValidation).length;
const hasTestFiles = testFileCount > 0;
const hasValidation = passedValidationCount > 0;
const testEvidenceLevel = hasTestFiles && hasValidation ? "both" : hasTestFiles ? "test_files" : hasValidation ? "validation_commands" : "none";
const testCoverage = buildTestCoverageSummary(changedPaths);
const scorer = args.input.localScorer;

return {
Expand All @@ -81,6 +84,7 @@
passedValidationCount,
commands: validation,
},
testCoverage,
linkedIssues: [...(args.input.linkedIssues ?? [])].sort((left, right) => left - right),
baseFreshness: args.analysis.baseFreshness,
ciStatusHints: [...(args.input.ciStatusHints ?? [])],
Expand Down
41 changes: 39 additions & 2 deletions src/signals/slop.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { GENERIC_COMMIT_PATTERN, hasClearNoIssueRationale, type SignalFinding } from "./engine";

Check notice on line 1 in src/signals/slop.ts

View check run for this annotation

Loopover ORB / Gittensory Context

Review queue is busy

This repo has a busy review queue in the local Gittensory cache.
import { isCodeFile, isTestFile } from "./local-branch";
import { hasLocalTestEvidence, isTestPath } from "./test-evidence";
import { buildTestCoverageSummary, hasLocalTestEvidence, isTestEvidencePath, isTestPath } from "./test-evidence";
import { isFocusManifestPublicSafe } from "./focus-manifest";
import { classifyChangedFile } from "./path-matchers";

Expand Down Expand Up @@ -47,6 +47,7 @@
export const SLOP_WEIGHTS = {
trivialWhitespaceChurn: 30,
missingTestEvidence: 15,
weakTestCoverage: 10,
nonSubstantivePadding: 30,
emptyDescription: 15,
lowQualityCommitMessage: 15,
Expand All @@ -65,6 +66,7 @@
"Current deterministic signals:",
"- trivial / whitespace-only churn",
"- missing test evidence",
"- weak test coverage (some tests, but disproportionately low for the source diff)",
"- non-substantive padding (generated / vendored / minified output as source)",
"- empty pull request description on a code change",
"- generic or empty commit message",
Expand All @@ -86,13 +88,15 @@
const findings: SignalFinding[] = [];
const trivialChurnFinding = buildTrivialWhitespaceChurnFinding(input);
const missingTestEvidenceFinding = buildMissingTestEvidenceFinding(input);
const weakTestCoverageFinding = buildWeakTestCoverageFinding(input);
const nonSubstantivePaddingFinding = buildNonSubstantivePaddingFinding(input);
const emptyDescriptionFinding = buildEmptyDescriptionFinding(input);
const lowQualityCommitMessageFinding = buildLowQualityCommitMessageFinding(input);
const duplicateClusterFinding = buildDuplicateClusterFinding(input);
const noLinkedIssueRationaleFinding = buildNoLinkedIssueRationaleFinding(input);
if (trivialChurnFinding) findings.push(trivialChurnFinding);
if (missingTestEvidenceFinding) findings.push(missingTestEvidenceFinding);
if (weakTestCoverageFinding) findings.push(weakTestCoverageFinding);
if (nonSubstantivePaddingFinding) findings.push(nonSubstantivePaddingFinding);
if (emptyDescriptionFinding) findings.push(emptyDescriptionFinding);
if (lowQualityCommitMessageFinding) findings.push(lowQualityCommitMessageFinding);
Expand All @@ -102,6 +106,7 @@
const slopRisk = clamp(
(trivialChurnFinding ? SLOP_WEIGHTS.trivialWhitespaceChurn : 0) +
(missingTestEvidenceFinding ? SLOP_WEIGHTS.missingTestEvidence : 0) +
(weakTestCoverageFinding ? SLOP_WEIGHTS.weakTestCoverage : 0) +
(nonSubstantivePaddingFinding ? SLOP_WEIGHTS.nonSubstantivePadding : 0) +
(emptyDescriptionFinding ? SLOP_WEIGHTS.emptyDescription : 0) +
(lowQualityCommitMessageFinding ? SLOP_WEIGHTS.lowQualityCommitMessage : 0) +
Expand Down Expand Up @@ -262,7 +267,7 @@
// per-file line counts are unavailable we trust the path (can't prove emptiness); when known, require a few
// added lines so a stub can't fake coverage. (#audit-3.1)
const hasSubstantiveTestFile = changedFiles.some((file) => {
if (!(isTestFile(file.path) || isTestPath(file.path))) return false;
if (!(isTestFile(file.path) || isTestEvidencePath(file.path))) return false;
return file.additions === undefined || nonNegative(file.additions) >= MIN_SUBSTANTIVE_TEST_ADDITIONS;
});
const hasChangedTestPaths = hasSubstantiveTestFile || hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles });
Expand All @@ -287,6 +292,38 @@
};
}

export function buildWeakTestCoverageFinding(input: SlopAssessmentInput): SignalFinding | null {
const changedFiles = input.changedFiles ?? [];
const changedPaths = changedFiles.map((file) => file.path).filter(Boolean);
const coverage = buildTestCoverageSummary(changedPaths);
if (coverage.classification !== "weak") return null;
if (coverage.sourcePathCount === 0) return null;

const hasSubstantiveTestFile = changedFiles.some((file) => {
if (!isTestEvidencePath(file.path)) return false;
return file.additions === undefined || nonNegative(file.additions) >= MIN_SUBSTANTIVE_TEST_ADDITIONS;
});
if (!hasSubstantiveTestFile && !hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })) return null;

const detail = ensurePublicSafeText(
`Changed paths include ${coverage.sourcePathCount} source file(s) but only ${coverage.testPathCount + coverage.fixturePathCount} test/fixture path(s) (${Math.round(coverage.testToChangedRatio * 100)}% of the diff).`,
"Source changes outnumber accompanying test or fixture evidence.",
);
const action = ensurePublicSafeText(
coverage.guidance,
"Add focused regression tests or fixtures for the touched modules.",
);

return {
code: "weak_test_coverage",
title: "Test coverage is disproportionately weak",
severity: "info",
detail,
action,
publicText: detail,
};
}

export function buildTrivialWhitespaceChurnFinding(input: SlopAssessmentInput): SignalFinding | null {
const changedFiles = input.changedFiles ?? [];
const lineTotals = summarizeChangedLines(changedFiles);
Expand Down
Loading
Loading