Skip to content

refactor: deduplicate clampInt utility across convex modules #1533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
goulonghui wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: deduplicate clampInt utility across convex modules #1533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 1 addition & 3 deletions convex/commentModeration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Id } from "./_generated/dataModel";
import type { ActionCtx, MutationCtx } from "./_generated/server";
import { action, internalAction, internalMutation, internalQuery } from "./functions";
import { assertRole, requireUserFromAction } from "./lib/access";
import { clampInt } from "./lib/math";
import {
buildCommentScamBanReason,
isCertainScam,
Expand Down Expand Up @@ -474,6 +475,3 @@ export const scheduleCommentScamModeration: ReturnType<typeof action> = action({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.min(Math.max(Math.trunc(value), min), max);
}
4 changes: 1 addition & 3 deletions convex/githubBackups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { internal } from "./_generated/api";
import type { Doc, Id } from "./_generated/dataModel";
import { action, internalMutation, internalQuery } from "./functions";
import { assertRole, requireUserFromAction } from "./lib/access";
import { clampInt } from "./lib/math";

const DEFAULT_BATCH_SIZE = 50;
const MAX_BATCH_SIZE = 200;
Expand Down Expand Up @@ -186,6 +187,3 @@ export const syncGitHubBackups: ReturnType<typeof action> = action({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, Math.floor(value)));
}
4 changes: 1 addition & 3 deletions convex/githubBackupsNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { internal } from "./_generated/api";
import type { Doc } from "./_generated/dataModel";
import type { ActionCtx } from "./_generated/server";
import { internalAction } from "./functions";
import { clampInt } from "./lib/math";
import {
backupSkillToGitHub,
deleteGitHubSkillBackup,
Expand Down Expand Up @@ -317,6 +318,3 @@ export const deleteGitHubBackupForSlugInternal = internalAction({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, Math.floor(value)));
}
4 changes: 1 addition & 3 deletions convex/githubSoulBackups.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { internal } from "./_generated/api";
import type { Doc, Id } from "./_generated/dataModel";
import { action, internalMutation, internalQuery } from "./functions";
import { assertRole, requireUserFromAction } from "./lib/access";
import { clampInt } from "./lib/math";

const DEFAULT_BATCH_SIZE = 50;
const MAX_BATCH_SIZE = 200;
Expand Down Expand Up @@ -165,6 +166,3 @@ export const syncGitHubSoulBackups: ReturnType<typeof action> = action({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, Math.floor(value)));
}
4 changes: 1 addition & 3 deletions convex/githubSoulBackupsNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { internal } from "./_generated/api";
import type { Doc } from "./_generated/dataModel";
import type { ActionCtx } from "./_generated/server";
import { internalAction } from "./functions";
import { clampInt } from "./lib/math";
import {
backupSoulToGitHub,
fetchGitHubSoulMeta,
Expand Down Expand Up @@ -181,6 +182,3 @@ export const syncGitHubSoulBackupsInternal = internalAction({
handler: syncGitHubSoulBackupsInternalHandler,
});

function clampInt(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, Math.floor(value)));
}
4 changes: 1 addition & 3 deletions convex/leaderboards.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { v } from "convex/values";
import { internal } from "./_generated/api";
import { internalAction, internalMutation, internalQuery } from "./functions";
import { clampInt } from "./lib/math";
import {
buildTrendingEntriesFromDailyRows,
getTrendingRange,
Expand Down Expand Up @@ -136,6 +137,3 @@ export const rebuildTrendingLeaderboardInternal = internalMutation({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.min(Math.max(value, min), max);
}
35 changes: 35 additions & 0 deletions convex/lib/math.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { describe, expect, it } from "vitest";
import { clampInt } from "./math";

describe("clampInt", () => {
it("clamps within range", () => {
expect(clampInt(5, 1, 10)).toBe(5);
});

it("clamps below min", () => {
expect(clampInt(-3, 1, 10)).toBe(1);
});

it("clamps above max", () => {
expect(clampInt(20, 1, 10)).toBe(10);
});

it("truncates toward zero", () => {
expect(clampInt(3.9, 1, 10)).toBe(3);
expect(clampInt(-2.9, -5, 5)).toBe(-2);
});

it("returns min for NaN", () => {
expect(clampInt(NaN, 1, 10)).toBe(1);
});

it("returns min for Infinity", () => {
expect(clampInt(Infinity, 1, 10)).toBe(1);
expect(clampInt(-Infinity, 1, 10)).toBe(1);
});

it("handles exact boundaries", () => {
expect(clampInt(1, 1, 10)).toBe(1);
expect(clampInt(10, 1, 10)).toBe(10);
});
});
11 changes: 11 additions & 0 deletions convex/lib/math.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Clamp a numeric value to an integer within [min, max].
*
* Truncates toward zero (like `Math.trunc`), handles NaN / ±Infinity by
* falling back to `min`, and clamps the result to the given range.
*/
export function clampInt(value: number, min: number, max: number): number {
const truncated = Math.trunc(value);
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent Math.round → Math.trunc behavior change for skills.ts and souls.ts

The old clampInt in convex/skills.ts and convex/souls.ts used Math.round, so a float like 3.9 would produce 4; Math.trunc(3.9) returns 3. All call sites in those two files are pagination limits (args.limit, args.numItems) backed by v.optional(v.number()) validators — meaning any float is a valid input. A caller passing limit: 5.9 previously received up to 6 results and will now receive 5. Please confirm the switch to Math.trunc is intentional for these modules, or restore rounding at the affected call sites.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: convex/lib/math.ts
Line: 8

Comment:
**Silent `Math.round` → `Math.trunc` behavior change for skills.ts and souls.ts**

The old `clampInt` in `convex/skills.ts` and `convex/souls.ts` used `Math.round`, so a float like `3.9` would produce `4`; `Math.trunc(3.9)` returns `3`. All call sites in those two files are pagination limits (`args.limit`, `args.numItems`) backed by `v.optional(v.number())` validators — meaning any float is a valid input. A caller passing `limit: 5.9` previously received up to 6 results and will now receive 5. Please confirm the switch to `Math.trunc` is intentional for these modules, or restore rounding at the affected call sites.

How can I resolve this? If you propose a fix, please make it concise.

if (!Number.isFinite(truncated)) return min;
return Math.min(max, Math.max(min, truncated));
}
6 changes: 1 addition & 5 deletions convex/maintenance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Doc, Id } from "./_generated/dataModel";
import type { ActionCtx } from "./_generated/server";
import { action, internalAction, internalMutation, internalQuery } from "./functions";
import { assertRole, requireUserFromAction } from "./lib/access";
import { clampInt } from "./lib/math";
import { buildSkillSummaryBackfillPatch, type ParsedSkillData } from "./lib/skillBackfill";
import {
computeQualitySignals,
Expand Down Expand Up @@ -2066,8 +2067,3 @@ export const backfillDigestIsSuspicious = internalMutation({
},
});

function clampInt(value: number, min: number, max: number) {
const rounded = Math.trunc(value);
if (!Number.isFinite(rounded)) return min;
return Math.min(max, Math.max(min, rounded));
}
5 changes: 1 addition & 4 deletions convex/skills.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
query,
} from "./functions";
import { assertAdmin, assertModerator, requireUser, requireUserFromAction } from "./lib/access";
import { clampInt } from "./lib/math";
import { getSkillBadgeMap, getSkillBadgeMaps, isSkillHighlighted } from "./lib/badges";
import { scheduleNextBatchIfNeeded } from "./lib/batching";
import { generateChangelogPreview as buildChangelogPreview } from "./lib/changelog";
Expand Down Expand Up @@ -6340,10 +6341,6 @@ export const setSkillSoftDeletedInternal = internalMutation({
},
});

function clampInt(value: number, min: number, max: number) {
const rounded = Number.isFinite(value) ? Math.round(value) : min;
return Math.min(max, Math.max(min, rounded));
}

async function findCanonicalSkillForFingerprint(
ctx: { db: MutationCtx["db"] },
Expand Down
5 changes: 1 addition & 4 deletions convex/souls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { embeddingVisibilityFor } from "./lib/embeddingVisibility";
import { toPublicSoul, toPublicUser } from "./lib/public";
import { getFrontmatterValue, hashSkillFiles } from "./lib/skills";
import { generateSoulChangelogPreview } from "./lib/soulChangelog";
import { clampInt } from "./lib/math";
import { fetchText, type PublishResult, publishSoulVersionForUser } from "./lib/soulPublish";

export { publishSoulVersionForUser } from "./lib/soulPublish";
Expand Down Expand Up @@ -671,7 +672,3 @@ export const setSoulSoftDeletedInternal = internalMutation({
},
});

function clampInt(value: number, min: number, max: number) {
const rounded = Number.isFinite(value) ? Math.round(value) : min;
return Math.min(max, Math.max(min, rounded));
}
4 changes: 1 addition & 3 deletions convex/statsMaintenance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { internal } from "./_generated/api";
import type { Doc } from "./_generated/dataModel";
import type { ActionCtx } from "./_generated/server";
import { internalAction, internalMutation, internalQuery } from "./functions";
import { clampInt } from "./lib/math";
import {
countPublicSkillsForGlobalStats,
isPublicSkillDoc,
Expand Down Expand Up @@ -301,9 +302,6 @@ export const runReconcileSkillStarCountsInternal = internalAction({
},
});

function clampInt(value: number, min: number, max: number) {
return Math.min(Math.max(value, min), max);
}

/**
* Count a page of skillSearchDigest docs and return the partial public count.
Expand Down
4 changes: 1 addition & 3 deletions convex/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getUserByHandleOrPersonalPublisher,
} from "./lib/publishers";
import { toPublicUser } from "./lib/public";
import { clampInt } from "./lib/math";
import {
getLatestActiveReservedHandle,
isHandleReservedForAnotherUser,
Expand Down Expand Up @@ -424,9 +425,6 @@ async function queryUsersForAdminList(
};
}

function clampInt(value: number, min: number, max: number) {
return Math.min(Math.max(Math.trunc(value), min), max);
}

export const getByHandle = query({
args: { handle: v.string() },
Expand Down