diff --git a/packages/gittensory-engine/package.json b/packages/gittensory-engine/package.json index 1505664f7e..13f188a116 100644 --- a/packages/gittensory-engine/package.json +++ b/packages/gittensory-engine/package.json @@ -46,6 +46,10 @@ "./signals/test-evidence": { "types": "./dist/signals/test-evidence.d.ts", "default": "./dist/signals/test-evidence.js" + }, + "./signals/check-summary": { + "types": "./dist/signals/check-summary.d.ts", + "default": "./dist/signals/check-summary.js" } }, "files": [ @@ -54,7 +58,7 @@ ], "scripts": { "build": "tsc -p tsconfig.json", - "test": "npm run build && tsc -p tsconfig.test.json && node --test \"dist-test/**/*.test.js\"" + "test": "npm run build && rm -rf dist-test && tsc -p tsconfig.test.json && node --test \"dist-test/**/*.test.js\"" }, "dependencies": { "yaml": "^2.9.0" diff --git a/packages/gittensory-engine/src/reward-risk.ts b/packages/gittensory-engine/src/reward-risk.ts index a4e981b3d9..464c30f155 100644 --- a/packages/gittensory-engine/src/reward-risk.ts +++ b/packages/gittensory-engine/src/reward-risk.ts @@ -3,14 +3,15 @@ // // Unlike the earlier self-contained extractions, reward-risk sits on top of the maintainer signal stack in // `src/signals/engine.ts` (`buildRoleContext`, `buildLaneAdvice`, `buildCollisionReport`, `buildQueueHealth`, -// `buildRepoFitRecommendation`, `buildContributorIntakeHealth`, `buildPullRequestReviewIntelligence`) and on -// `isFailingCheckSummary` from `src/signals/local-branch.ts`. Those builders are not yet extracted (and are -// far too large to port under the per-issue size cap), so — rather than reach back into `src/`, which this -// package must never do — they are DEPENDENCY-INJECTED via `RewardRiskEngineDeps`. The -// `src/signals/reward-risk.ts` shim binds the real `src` builders; a follow-up issue can drop the injection -// once they have engine homes. +// `buildRepoFitRecommendation`, `buildContributorIntakeHealth`, `buildPullRequestReviewIntelligence`) from +// `src/signals/engine.ts`. Those builders are not yet extracted (and are far too large to port under the +// per-issue size cap), so — rather than reach back into `src/`, which this package must never do — they are +// DEPENDENCY-INJECTED via `RewardRiskEngineDeps`. `isFailingCheckSummary` now lives in +// `./signals/check-summary.js` (#4256). The `src/signals/reward-risk.ts` shim binds the real `src` builders; +// a follow-up issue can drop the injection once they have engine homes. import type { ScorePreviewResult } from "./scoring/preview.js"; import { buildScorePreview } from "./scoring/preview.js"; +import { isFailingCheckSummary } from "./signals/check-summary.js"; import { nowIso } from "./utils/json.js"; import type { CheckSummaryRecord, @@ -60,10 +61,10 @@ export type PullRequestReviewabilityInput = { }; /** - * The `src/signals/engine.ts` + `src/signals/local-branch.ts` builders reward-risk depends on, injected so - * this package stays free of any `src/` import. The real `src`-typed builders bind cleanly: their argument - * records are wider than (assignable from) these engine mirrors, and their richer return types are - * covariantly assignable to the narrowed views above. + * The `src/signals/engine.ts` builders reward-risk depends on, injected so this package stays free of any + * `src/` import. The real `src`-typed builders bind cleanly: their argument records are wider than (assignable + * from) these engine mirrors, and their richer return types are covariantly assignable to the narrowed views + * above. `isFailingCheckSummary` is imported directly from `./signals/check-summary.js` (#4256). */ export type RewardRiskEngineDeps = { buildRoleContext: (args: { @@ -104,7 +105,6 @@ export type RewardRiskEngineDeps = { collisions: CollisionReport, ) => { level: "healthy" | "watch" | "strained" | "blocked" }; buildPullRequestReviewIntelligence: (args: PullRequestReviewabilityInput) => PullRequestReviewIntelligenceView; - isFailingCheckSummary: (check: CheckSummaryRecord) => boolean; }; export type RewardRiskActionKind = @@ -541,7 +541,7 @@ export function buildMaintainerNoiseReport( export function buildPullRequestReviewability(args: PullRequestReviewabilityInput, deps: RewardRiskEngineDeps): PullRequestReviewability { const intelligence = deps.buildPullRequestReviewIntelligence(args); const pr = args.pullRequest; - const failingChecks = args.checks.filter(deps.isFailingCheckSummary).length; + const failingChecks = args.checks.filter(isFailingCheckSummary).length; const broadDiff = intelligence.changeSummary.fileCount >= 12 || intelligence.changeSummary.additions + intelligence.changeSummary.deletions >= 800; const noiseSources = [ ...(pr?.state && pr.state !== "open" ? [`PR is ${pr.state}.`] : []), diff --git a/packages/gittensory-engine/src/signals/check-summary.ts b/packages/gittensory-engine/src/signals/check-summary.ts new file mode 100644 index 0000000000..adaffc7a38 --- /dev/null +++ b/packages/gittensory-engine/src/signals/check-summary.ts @@ -0,0 +1,15 @@ +import type { CheckSummaryRecord } from "../scoring/types.js"; + +/** Conclusion/status values that mark a single cached check as failing or attention-needing. */ +const FAILING_CHECK_STATES = ["failure", "failed", "timed_out", "cancelled", "action_required", "startup_failure"]; + +/** + * Canonical "is this ONE cached check failing?" predicate, shared so every surface (readiness, the maintainer + * queue digest, reward-risk reviewability) classifies a check identically. A cached check may carry its outcome + * on `conclusion` (check runs) OR only on `status` (commit-status rows and runs that errored before concluding), + * so fall back to `status` when `conclusion` is absent, and case-fold both — GitHub conclusions are lowercase, + * but cached/commit statuses are not guaranteed to be. + */ +export function isFailingCheckSummary(check: CheckSummaryRecord): boolean { + return FAILING_CHECK_STATES.includes((check.conclusion ?? check.status).toLowerCase()); +} diff --git a/packages/gittensory-engine/test/check-summary.test.ts b/packages/gittensory-engine/test/check-summary.test.ts new file mode 100644 index 0000000000..57d0385e0b --- /dev/null +++ b/packages/gittensory-engine/test/check-summary.test.ts @@ -0,0 +1,33 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { isFailingCheckSummary } from "../dist/signals/check-summary.js"; +import type { CheckSummaryRecord } from "../dist/scoring/types.js"; + +function check(overrides: Partial & Pick): CheckSummaryRecord { + return { + id: "check-1", + repoFullName: "owner/repo", + name: "ci", + payload: {}, + ...overrides, + }; +} + +test("isFailingCheckSummary: treats known failing conclusions as failing", () => { + for (const conclusion of ["failure", "failed", "timed_out", "cancelled", "action_required", "startup_failure"]) { + assert.equal(isFailingCheckSummary(check({ status: "completed", conclusion })), true, conclusion); + assert.equal(isFailingCheckSummary(check({ status: conclusion.toUpperCase(), conclusion: null })), true, `${conclusion} on status`); + } +}); + +test("isFailingCheckSummary: treats success-like outcomes as not failing", () => { + assert.equal(isFailingCheckSummary(check({ status: "completed", conclusion: "success" })), false); + assert.equal(isFailingCheckSummary(check({ status: "success", conclusion: null })), false); + assert.equal(isFailingCheckSummary(check({ status: "in_progress", conclusion: null })), false); +}); + +test("isFailingCheckSummary: falls back to status when conclusion is absent", () => { + assert.equal(isFailingCheckSummary(check({ status: "FAILED", conclusion: undefined })), true); + assert.equal(isFailingCheckSummary(check({ status: "SUCCESS", conclusion: undefined })), false); +}); diff --git a/src/github/commands.ts b/src/github/commands.ts index 10e2f95206..57d6b9d142 100644 --- a/src/github/commands.ts +++ b/src/github/commands.ts @@ -22,7 +22,7 @@ import { type QueueHealth, type RepoOutcomePatterns, } from "../signals/engine"; -import { isFailingCheckSummary } from "../signals/local-branch"; +import { isFailingCheckSummary } from "../signals/check-summary"; import { buildMaintainerNoiseReport, type MaintainerNoiseReport } from "../signals/reward-risk"; const PUBLIC_MENTION_COMMAND_CATALOG = [ diff --git a/src/signals/check-summary.ts b/src/signals/check-summary.ts new file mode 100644 index 0000000000..6db8e0246b --- /dev/null +++ b/src/signals/check-summary.ts @@ -0,0 +1,7 @@ +// Check-summary classifiers, extracted to `@jsonbored/gittensory-engine` (#4256) so reward-risk and the +// published gittensory-mcp/gittensory-miner CLIs can depend on the same source instead of reaching into +// `local-branch.ts` (which pulls in the whole review-scoring/Gittensor-API subsystem). This file is a thin +// re-export shim; the implementation lives at packages/gittensory-engine/src/signals/check-summary.ts +// (imported via relative source path, not the published package, to match this repo's existing +// engine-consumption convention — see e.g. src/signals/test-evidence.ts). +export * from "../../packages/gittensory-engine/src/signals/check-summary"; diff --git a/src/signals/contributor-open-pr-monitor.ts b/src/signals/contributor-open-pr-monitor.ts index b574d5a159..b32d6475b4 100644 --- a/src/signals/contributor-open-pr-monitor.ts +++ b/src/signals/contributor-open-pr-monitor.ts @@ -11,7 +11,8 @@ import { import type { CheckSummaryRecord, PullRequestFileRecord, PullRequestRecord, PullRequestReviewRecord } from "../types"; import { nowIso } from "../utils/json"; import { buildRoleContext } from "./engine"; -import { isCodeFile, isFailingCheckSummary } from "./local-branch"; +import { isFailingCheckSummary } from "./check-summary"; +import { isCodeFile } from "./local-branch"; import { isTestPath } from "./test-evidence"; export type OpenPrWorkClassification = diff --git a/src/signals/engine.ts b/src/signals/engine.ts index dfb79f23df..8f136910dd 100644 --- a/src/signals/engine.ts +++ b/src/signals/engine.ts @@ -29,7 +29,7 @@ import { sanitizePublicComment } from "../queue-intelligence"; import { labelMatchesPattern, projectLinkedIssueMultiplierForPlannedSolve, type LinkedIssueMultiplierStatus } from "../scoring/preview"; import { hasLocalTestEvidence, hasValidationNote, isTestPath } from "./test-evidence"; import { isCodeFile, isTestFile } from "./path-matchers"; -import { isFailingCheckSummary } from "./local-branch"; +import { isFailingCheckSummary } from "./check-summary"; import { isDuplicateClusterWinnerByClaim } from "./duplicate-winner"; import { PREFLIGHT_LIMITS } from "./preflight-limits"; import type { UnifiedCollapsible } from "../review/unified-comment"; diff --git a/src/signals/local-branch.ts b/src/signals/local-branch.ts index dcf77c9421..60c4a584d1 100644 --- a/src/signals/local-branch.ts +++ b/src/signals/local-branch.ts @@ -3,6 +3,8 @@ import { buildScorePreview } from "../scoring/preview"; import type { GittensorContributorSnapshot } from "../gittensor/api"; import type { BountyRecord, CheckSummaryRecord, IssueRecord, PullRequestRecord, RecentMergedPullRequestRecord, RepositoryRecord, ScoringModelSnapshotRecord } from "../types"; import { nowIso } from "../utils/json"; +import { isFailingCheckSummary } from "./check-summary"; +export { isFailingCheckSummary } from "./check-summary"; import { buildCollisionReport, buildLaneAdvice, @@ -750,20 +752,6 @@ function matchingCheckSummaries(pr: PullRequestRecord, checkSummaries: CheckSumm ); } -/** Conclusion/status values that mark a single cached check as failing or attention-needing. */ -const FAILING_CHECK_STATES = ["failure", "failed", "timed_out", "cancelled", "action_required", "startup_failure"]; - -/** - * Canonical "is this ONE cached check failing?" predicate, shared so every surface (readiness, the maintainer - * queue digest) classifies a check identically. A cached check may carry its outcome on `conclusion` (check - * runs) OR only on `status` (commit-status rows and runs that errored before concluding), so fall back to - * `status` when `conclusion` is absent, and case-fold both — GitHub conclusions are lowercase, but cached/commit - * statuses are not guaranteed to be. - */ -export function isFailingCheckSummary(check: CheckSummaryRecord): boolean { - return FAILING_CHECK_STATES.includes((check.conclusion ?? check.status).toLowerCase()); -} - function hasFailingCheck(checks: CheckSummaryRecord[]): boolean { return checks.some(isFailingCheckSummary); } diff --git a/src/signals/reward-risk.ts b/src/signals/reward-risk.ts index 50db994227..f9eac74b80 100644 --- a/src/signals/reward-risk.ts +++ b/src/signals/reward-risk.ts @@ -7,8 +7,9 @@ // This is a WRAPPING shim rather than the usual pure `export *` re-export. reward-risk depends on the // maintainer signal stack in `src/signals/engine.ts` (`buildRoleContext`, `buildLaneAdvice`, // `buildCollisionReport`, `buildQueueHealth`, `buildRepoFitRecommendation`, `buildContributorIntakeHealth`, -// `buildPullRequestReviewIntelligence`) plus `isFailingCheckSummary` from `./local-branch` — none of which -// are extracted yet (and are far too large to port under the size cap). The engine module takes them as an +// `buildPullRequestReviewIntelligence`) — none of which are extracted yet (and are far too large to port +// under the size cap). `isFailingCheckSummary` now lives in `@jsonbored/gittensory-engine` (#4256). The +// engine module takes the remaining builders as an // injected `RewardRiskEngineDeps`; this shim binds the real `src` builders and threads them in, so every // existing importer keeps calling the four builders with their original signatures. Once those builders gain // engine homes, a follow-up can drop the injection and collapse this back to a plain re-export. @@ -28,8 +29,6 @@ import { buildRepoFitRecommendation, buildRoleContext, } from "./engine"; -import { isFailingCheckSummary } from "./local-branch"; - export type { ContributorRewardRiskStrategy, EligibilityGapEntry, @@ -54,7 +53,6 @@ const deps: RewardRiskEngineDeps = { buildRepoFitRecommendation, buildContributorIntakeHealth, buildPullRequestReviewIntelligence, - isFailingCheckSummary, }; export function buildRepoRewardRisk(