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
26 changes: 24 additions & 2 deletions src/scoring/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ function computeScoreCore(
const totalTokenScore =
input.totalTokenScore === undefined ? nonNegative(derivedTotalTokenScore) : applyNonCodeCapToTotal(input.totalTokenScore, input, cappedNonCodeTokenScore);
const sourceLines = Math.max(1, nonNegative(input.sourceLines ?? sourceTokenScore));
const fixedBaseScore = input.fixedBaseScore ?? config?.fixedBaseScore ?? undefined;
const fixedBaseScore = clampFixedBaseScore(input.fixedBaseScore ?? config?.fixedBaseScore);
const rawDensity = sourceTokenScore / sourceLines;
// Density branch (#812): upstream is on the saturation model, but `current_density_model` is still a
// supported `activeModel` (types.ts union, the public OpenAPI schema, the DB parser, ~20 test fixtures, and
Expand Down Expand Up @@ -1261,7 +1261,11 @@ export function calculateTimeDecay(prAgeHours: number, constants: Record<string,
}

function saturationScore(sourceTokenScore: number, totalTokenScore: number, constants: Record<string, number>): number {
const scale = Math.max(constant(constants, "SRC_TOK_SATURATION_SCALE"), 1);
// SRC_TOK_SATURATION_SCALE is per-repo overridable only within [10, 500] upstream; a snapshot value outside
// that band (a bad override, a parse glitch) would otherwise distort the saturation curve — at scale 1 the
// component saturates almost immediately, well above the documented floor. Clamp to the documented range so
// the curve stays within upstream bounds (the prior Math.max(...,1) only guarded the divide-by-zero edge).
const scale = clampSaturationScale(constant(constants, "SRC_TOK_SATURATION_SCALE"));
return (
constant(constants, "MERGED_PR_BASE_SCORE") * (1 - Math.exp(-sourceTokenScore / scale)) +
saturationContributionBonus(totalTokenScore, constants)
Expand Down Expand Up @@ -1291,6 +1295,24 @@ function clamp(value: number, min: number, max: number): number {
return Math.min(max, Math.max(min, value));
}

// Bounds documented by upstream for the two repo-configurable scoring inputs the preview consumes directly.
const FIXED_BASE_SCORE_MIN = 0;
const FIXED_BASE_SCORE_MAX = 100;
const SRC_TOK_SATURATION_SCALE_MIN = 10;
const SRC_TOK_SATURATION_SCALE_MAX = 500;

// A repo's fixed_base_score override forces base_score to a constant within [0, 100]. The value reaches the
// preview unbounded above — the API schema only enforces `.min(0)` and registry normalization accepts any
// finite number — so a misconfigured 150 would otherwise mint a base score above the model ceiling. Clamp to
// the documented range; a non-finite/absent value falls through to the token-derived base score.
function clampFixedBaseScore(value: number | null | undefined): number | undefined {
return Number.isFinite(value) ? clamp(value as number, FIXED_BASE_SCORE_MIN, FIXED_BASE_SCORE_MAX) : undefined;
}

function clampSaturationScale(value: number): number {
return clamp(value, SRC_TOK_SATURATION_SCALE_MIN, SRC_TOK_SATURATION_SCALE_MAX);
}

function roundScore(value: number): number {
return Math.round(value * 10000) / 10000;
}
41 changes: 41 additions & 0 deletions test/unit/scoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1848,4 +1848,45 @@ NOVELTY_BONUS_SCALAR = 3
).toEqual([]);
});
});

describe("documented scoring config bounds (#1744)", () => {
const saturationBase = (scale: number): number =>
buildScorePreview({
repo,
snapshot: { ...snapshot, activeModel: "pending_saturation_model" as const, constants: { ...snapshot.constants, SRC_TOK_SATURATION_SCALE: scale } },
input: { repoFullName: repo.fullName, sourceTokenScore: 30, totalTokenScore: 30, sourceLines: 30, openPrCount: 0, credibility: 1 },
}).scoreEstimate.baseScore;

it("clamps a fixed_base_score above the documented ceiling to 100", () => {
const preview = buildScorePreview({
repo: { ...repo, registryConfig: { ...repo.registryConfig!, fixedBaseScore: 150 } },
snapshot,
input: { repoFullName: repo.fullName, sourceTokenScore: 100, totalTokenScore: 200, sourceLines: 10, openPrCount: 0, credibility: 1 },
});
// Before the fix this previewed baseScore = 150 (API schema is `.min(0)` only; registry normalization
// accepts any finite value), minting a base component above the documented [0, 100] ceiling.
expect(preview.scoreEstimate.baseScore).toBe(100);
});

it("clamps a negative fixed_base_score (via the API input path) to 0", () => {
const preview = buildScorePreview({
repo,
snapshot,
input: { repoFullName: repo.fullName, fixedBaseScore: -5, sourceTokenScore: 100, totalTokenScore: 200, sourceLines: 10, openPrCount: 0, credibility: 1 },
});
expect(preview.scoreEstimate.baseScore).toBe(0);
});

it("clamps SRC_TOK_SATURATION_SCALE below the documented floor (10) before the saturation curve", () => {
// A scale of 3 is below the documented [10, 500] band, so it must score identically to the clamped
// floor of 10 — and differently from the in-band default 58 (the prior Math.max(...,1) left 3 in play).
expect(saturationBase(3)).toBe(saturationBase(10));
expect(saturationBase(3)).not.toBe(saturationBase(58));
});

it("clamps SRC_TOK_SATURATION_SCALE above the documented ceiling (500)", () => {
expect(saturationBase(1000)).toBe(saturationBase(500));
expect(saturationBase(1000)).not.toBe(saturationBase(400));
});
});
});
Loading