Skip to content
Merged
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
6 changes: 5 additions & 1 deletion packages/gittensory-engine/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand All @@ -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"
Expand Down
24 changes: 12 additions & 12 deletions packages/gittensory-engine/src/reward-risk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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}.`] : []),
Expand Down
15 changes: 15 additions & 0 deletions packages/gittensory-engine/src/signals/check-summary.ts
Original file line number Diff line number Diff line change
@@ -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());
}
33 changes: 33 additions & 0 deletions packages/gittensory-engine/test/check-summary.test.ts
Original file line number Diff line number Diff line change
@@ -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<CheckSummaryRecord> & Pick<CheckSummaryRecord, "status">): 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);
});
2 changes: 1 addition & 1 deletion src/github/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
7 changes: 7 additions & 0 deletions src/signals/check-summary.ts
Original file line number Diff line number Diff line change
@@ -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";
3 changes: 2 additions & 1 deletion src/signals/contributor-open-pr-monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/signals/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
16 changes: 2 additions & 14 deletions src/signals/local-branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 3 additions & 5 deletions src/signals/reward-risk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -28,8 +29,6 @@ import {
buildRepoFitRecommendation,
buildRoleContext,
} from "./engine";
import { isFailingCheckSummary } from "./local-branch";

export type {
ContributorRewardRiskStrategy,
EligibilityGapEntry,
Expand All @@ -54,7 +53,6 @@ const deps: RewardRiskEngineDeps = {
buildRepoFitRecommendation,
buildContributorIntakeHealth,
buildPullRequestReviewIntelligence,
isFailingCheckSummary,
};

export function buildRepoRewardRisk(
Expand Down