feat: new algorithms + organisation #2
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReview triggered.
|
WalkthroughAdds Prettier config and formatting scripts; updates documentation; introduces modular matchmaking algorithms (normal, dynamic, refactored ranked) with a registry and services abstraction; revises matchmaking orchestration (createMatch, getPartyMatch, discoverMatches); adjusts a gzip JSON middleware to always call next; widespread formatting changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Cron as Scheduler/Timer
participant MM as matchmaking.discoverMatches
participant Registry as algorithmRegistry
participant Algo as Algorithm (normal/dynamic/ranked)
participant Svc as Services
participant DB as MongoDB
Cron->>MM: Trigger discoverMatches(queueData)
MM->>Registry: lookup(queueData.queueType)
Registry-->>MM: Algorithm handler
MM->>Algo: execute(queueData, Services)
rect rgba(200,220,255,0.25)
note right of Algo: Algorithm core loop
Algo->>Svc: getOldestParties(queueId, limit)
Svc->>DB: query queues (oldest parties)
DB-->>Svc: parties[]
Algo->>Algo: packTeamsGreedy / range mgmt
alt match formed
Algo->>Svc: createMatch(queueData, teams, partiesUsed)
Svc->>DB: insert match, link foundParties, delete from queues
DB-->>Svc: ack
else expand search / stop
Algo->>Svc: updatePartyRange?(...) [ranked]
Svc->>DB: update party range
DB-->>Svc: ack
end
end
Algo-->>MM: done
sequenceDiagram
autonumber
participant Client
participant MW as parseGzippedJson
participant Next as Next Middleware/Route
Client->>MW: HTTP Request
alt content-type: application/json & content-encoding: gzip
MW->>MW: gunzip(body)
MW->>MW: JSON.parse(...)
MW->>Next: next()
else other content
MW->>Next: next() %% always proceeds
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/empty-handler.ts (1)
1-4: Stop using never + ts-ignore for a no-op handler; type it correctly.Returning an array from a function typed as () => never is unsound and currently masked by @ts-ignore. Use a void-returning handler (and accept the error param) to avoid breaking type inference and to drop the ts-ignore.
-// @ts-ignore -export const emptyHandler: () => never = () => []; +export const emptyHandler: (err?: unknown) => void = () => {};
🧹 Nitpick comments (46)
src/schemas/validQueues.ts (1)
5-5: Ensure ValidQueueId infers a literal union (not just string).If
validQueueIdsisn’tas const(and non-empty) where it’s defined,z.enum(validQueueIds)may widen types or fail tuple constraints. Since it’s imported from a.jsmodule, double-check the source exports a readonly tuple.In the module that defines
validQueueIds, consider:// modules/config.ts export const validQueueIds = ['normal', 'ranked', 'dynamic'] as const satisfies readonly [string, ...string[]];Then this file can also derive the union directly if desired:
export type ValidQueueId = typeof validQueueIds[number]; // (or keep z.infer as-is once the tuple typing is ensured).prettierrc (1)
1-5: Prettier config LGTM; consider pinning EOL to avoid cross-OS diffs.Optional: add
"endOfLine": "lf"to prevent CRLF/LF churn on Windows contributors.Apply:
{ "printWidth": 120, "singleQuote": false, - "trailingComma": "none" + "trailingComma": "none", + "endOfLine": "lf" }tests/configuration.ts (1)
1-1: Quote style change is fine; consider env-driven override for flexibility.-export const BASE_URL = "http://localhost:3000"; +export const BASE_URL = process.env.BASE_URL ?? "http://localhost:3000";src/middlewares/body-parser.ts (3)
14-21: Use try/catch and block-scoped vars for parse errors.Simplifies control flow and avoids mutable flag.
- return async (c: Context, next: Next) => { - var failedToParse = false; - const jsonBody = await c.req.json().catch(() => { - failedToParse = true; - }); - - if (failedToParse) { - return c.json({ error: "FailedToParseBody" }, 400); - } + return async (c: Context, next: Next) => { + let jsonBody: unknown; + try { + jsonBody = await c.req.json(); + } catch { + return c.json({ error: "FailedToParseBody" }, 400); + }
25-34: Optional: guard on Content-Type and standardize error shape.
- Return 415 when Content-Type isn’t JSON to fail fast.
- Keep a consistent error schema, e.g.,
{ error: "InvalidBody", details: [...] }.- if (schema) { + // Enforce JSON content-type (optional) + const ct = c.req.header("content-type")?.toLowerCase() ?? ""; + if (ct && !ct.includes("application/json")) { + return c.json({ error: "UnsupportedMediaType" }, 415); + } + + if (schema) { const parseResult = schema.safeParse(jsonBody); if (!parseResult.success) { - return c.json({ error: parseResult.error.errors }, 400); + return c.json({ error: "InvalidBody", details: parseResult.error.errors }, 400); }
36-39: Minor: return downstream response explicitly.Not required, but
return await next();mirrors common Hono style.- await next(); - return; + return await next();src/schemas/joinQueue.ts (1)
5-10: Tighten input validation (IDs, tokens, rankedValue).Small schema hardening to prevent empty/invalid values and accidental duplicates.
export const JoinQueueSchema = z.object({ - partyId: z.string().min(1), - userIds: z.array(z.number()).min(1), + partyId: z.string().min(1, "partyId required"), + userIds: z.array(z.number().int().positive()).min(1).refine(a => new Set(a).size === a.length, "duplicate userIds"), queueId: ValidQueueIdSchema, - rankedValue: z.number().nullable().optional(), - serverAccessToken: z.string().nullable().optional() + rankedValue: z.number().finite().nullable().optional(), + serverAccessToken: z.string().min(1).optional() // drop nullable unless empty token is meaningful });If null must be allowed for serverAccessToken, use: z.string().min(1).nullable().optional().
src/routes/v1/healthcheck.ts (1)
5-9: Consider adding a readiness check and build info.Healthchecks are more useful if they expose dependencies’ status (DB ping) and version/commit for debugging.
Example:
routes.get("/", (c) => { - return c.json({ success: true }); + return c.json({ success: true, version: process.env.BUILD_SHA }); }); + +routes.get("/ready", async (c) => { + // e.g., ping database/queue; return 503 if unhealthy + return c.json({ ready: true }); +});src/middlewares/error-handler.ts (1)
4-6: Structure logs and avoid leaking details in prod; keep response generic.Add minimal structure and a request id (if available). Keep JSON body stable.
-export const errorHandler = (err: Error, c: Context) => { - console.error(err); - return c.json({ error: "Internal Server Error" }, 500); -}; +export const errorHandler = (err: Error, c: Context) => { + const reqId = c.get?.("requestId"); + console.error({ err, reqId }, "Unhandled error"); + return c.json({ error: "Internal Server Error", requestId: reqId }, 500); +};If you don’t have a requestId middleware, drop that field or add one later.
src/modules/database.ts (1)
10-13: Use type-only imports to reduce emitted JS.These are types; importing with
typeavoids unnecessary runtime imports.-import { QueueDocument } from "types/queueDocument.js"; -import { ServerDocument } from "types/serverDocument.js"; -import { FoundMatchDocument } from "types/foundMatchDocument.js"; -import { FoundPartyDocument } from "types/foundPartyDocument.js"; +import type { QueueDocument } from "types/queueDocument.js"; +import type { ServerDocument } from "types/serverDocument.js"; +import type { FoundMatchDocument } from "types/foundMatchDocument.js"; +import type { FoundPartyDocument } from "types/foundPartyDocument.js";src/modules/database-indexes.ts (1)
14-16: Formatting-only change looks good.No behavioral change. Optional: consider making createIndexes async and awaiting index creation so startup failures surface.
Example (outside selected lines):
-export function createIndexes() { +export async function createIndexes() { + await Promise.allSettled([ + queuesCollection.createIndex({ partyId: -1 }, { name: "partyId" }), + queuesCollection.createIndex({ queueId: -1 }, { name: "queueId" }), + queuesCollection.createIndex({ timeAdded: -1 }, { name: "timeAdded", expireAfterSeconds: QUEUE_EXPIRE_AFTER }), + serverIdsCollection.createIndex({ createdAt: -1 }, { name: "createdAt", expireAfterSeconds: SERVER_EXPIRE_AFTER }), + foundMatchesCollection.createIndex({ createdAt: -1 }, { name: "createdAt", expireAfterSeconds: MATCHES_EXPIRE_AFTER }), + foundPartiesCollection.createIndex({ createdAt: -1 }, { name: "createdAt", expireAfterSeconds: MATCHES_EXPIRE_AFTER }) + ]).catch(emptyHandler); +}package.json (2)
8-9: Add a non-mutating format check script.Helpful for CI and pre-commit.
"scripts": { "start": "bun src/index.ts", "build": "tsc", "dev": "bun --watch src/index.ts", - "format": "prettier --write ." + "format": "prettier --write .", + "format:check": "prettier --check ." },
12-13: Avoid "latest" in devDependencies to prevent surprise breakages.Pin @types/bun to a specific version (or use a caret range).
- "@types/bun": "latest", + "@types/bun": "^1.1.10",src/middlewares/logging.ts (5)
11-17: Use high-resolution timer; Date.now() is integer ms so decimals are meaningless.Switch to performance.now() for sub-ms precision.
- const start = Date.now(); + const start = performance.now(); await next(); - const end = Date.now(); - const elapsedTime = end - start; + const end = performance.now(); + const elapsedTime = end - start;
21-24: URL parsing may throw if c.req.url is relative on some adapters.Prefer c.req.path plus query, or provide a base URL.
- const url = new URL(c.req.url); - const pathWithParams = url.pathname + url.search; + let pathWithParams: string; + try { + const url = new URL(c.req.url, "http://localhost"); + pathWithParams = url.pathname + url.search; + } catch { + // Fallback for adapters where c.req.url is already a path + const q = new URLSearchParams(c.req.query()).toString(); + pathWithParams = c.req.path + (q ? `?${q}` : ""); + }
26-26: Truncation digits can be reduced.With ms timings, 2–3 decimals are enough; 4 is noisy.
- const elapsedMillisecondsLog = `(${truncateDecimals(elapsedTime, 4)} ms)`.padStart(25); + const elapsedMillisecondsLog = `(${truncateDecimals(elapsedTime, 2)} ms)`.padStart(25);
28-29: Consider gating logs by environment.Reduce noise in production with a level check or DEBUG toggle.
- console.log(`${methodLog}${uriLog}${statusLog}${elapsedMillisecondsLog}`); + if (process.env.LOG_LEVEL !== "silent") { + console.log(`${methodLog}${uriLog}${statusLog}${elapsedMillisecondsLog}`); + }
4-7: truncateDecimals is fine; Math.trunc can simplify.Equivalent and clearer for truncation toward zero.
- const multiplier = Math.pow(10, digits); - const adjustedNum = number * multiplier; - const truncatedNum = Math[adjustedNum < 0 ? "ceil" : "floor"](adjustedNum); - return truncatedNum / multiplier; + const m = 10 ** digits; + return Math.trunc(number * m) / m;tests/manual/leave-queue.ts (3)
2-2: Remove unused import.
randomUUIDisn’t used.Apply:
-import { randomUUID } from "node:crypto";
12-15: Optional: Add Authorization header support for non-Testing envs.Read the token from env and forward it.
headers: { - "Content-Type": "application/json" + "Content-Type": "application/json", + // Optional: include auth if set + ...(process.env.AuthKey ? { Authorization: process.env.AuthKey } : {}) },
17-19: Handle non-2xx responses before parsing JSON.Prevents noisy JSON parse errors and surfaces HTTP status.
- .then((response) => response.json()) - .then((data) => console.log(data)) - .catch((error) => console.error("Error:", error)); + .then(async (response) => { + const text = await response.text(); + if (!response.ok) throw new Error(`HTTP ${response.status}: ${text}`); + return text ? JSON.parse(text) : null; + }) + .then((data) => console.log(data)) + .catch((error) => console.error("Error:", error));tests/manual/join-queue.ts (3)
6-7: Validate and sanitize user IDs input.Filters out NaNs and trims whitespace.
-const userIds = prompt("User IDs (comma-separated):")?.split(",").map(Number) || []; +const userIds = + prompt("User IDs (comma-separated):") + ?.split(",") + .map((s) => Number(s.trim())) + .filter((n) => Number.isFinite(n)) || [];
19-23: Optional: Forward Authorization header.Aligns manual calls with auth middleware when env ≠ Testing.
headers: { - "Content-Type": "application/json" + "Content-Type": "application/json", + ...(process.env.AuthKey ? { Authorization: process.env.AuthKey } : {}) },
25-27: Handle non-OK responses early.Consistent with leave-queue suggestion.
- .then((response) => response.json()) - .then((data) => console.log(data)) - .catch((error) => console.error("Error:", error)); + .then(async (response) => { + const text = await response.text(); + if (!response.ok) throw new Error(`HTTP ${response.status}: ${text}`); + return text ? JSON.parse(text) : null; + }) + .then((data) => console.log(data)) + .catch((error) => console.error("Error:", error));src/modules/config.ts (2)
6-6: Specify radix for parseInt.Avoids edge-case parsing issues.
-export const port = parseInt(process.env.Port || "3000"); +export const port = parseInt(process.env.Port || "3000", 10);
33-34: Derive validQueueIds from queues to avoid drift.Removes duplication and keeps types aligned.
-export const validQueueIds = ["Ranked2v2"] as const; +export type ValidQueueId = (typeof queues)[number]["queueId"]; +export const validQueueIds: readonly ValidQueueId[] = queues.map((q) => q.queueId);src/types/foundMatchDocument.ts (1)
9-13: Consider making fields readonly (and teams a fixed 2-team tuple if always 2v2).Improves type safety for persisted docs.
- teams: number[][]; - serverAccessToken: string; - queueId: string; - createdAt: Date; + readonly teams: number[][]; + readonly serverAccessToken: string; + readonly queueId: string; + readonly createdAt: Date;Optionally (if always two teams):
readonly teams: readonly [readonly number[], readonly number[]];src/routes/v1/join-queue.ts (3)
28-32: Simplify and harden rankedValue check
=== undefined || == nullis redundant. Also ensure numeric when schema allows.- if (queueConfig.queueType == "ranked") { - if (rankedValue === undefined || rankedValue == null) { + if (queueConfig.queueType === "ranked") { + if (rankedValue == null /* allow 0 */) { return c.json({ error: "Ranked value is required" }, 400); } }
36-43: Make serverAccessToken insert idempotent and avoid duplicate key noiseUse upsert instead of best-effort insert + catch. Optionally add TTL outside this PR.
- if (serverAccessToken) { - serverIdsCollection - .insertOne({ - _id: serverAccessToken, - createdAt: currentDate - }) - .catch(emptyHandler); - } + if (serverAccessToken) { + serverIdsCollection + .updateOne( + { _id: serverAccessToken }, + { $setOnInsert: { createdAt: currentDate } }, + { upsert: true } + ) + .catch(emptyHandler); + }
45-53: Possible race between pre-check and enqueueA match can be created between
getPartyMatchand the upsert, causing the party to be markedInQueuedespite a ready match. Consider re-checking after upsert when the doc was newly inserted.If desired, I can propose a small post-upsert recheck gated by
lastErrorObject.upsertedId.Also applies to: 54-74
docs/matchmaking.md (1)
39-41: Typo: ElapsedFix “timeElaspedToUseMinimumUsers” → “timeElapsedToUseMinimumUsers”.
-- Dynamic: `minUsersPerTeam`, `maxUsersPerTeam`, `timeElaspedToUseMinimumUsers` +- Dynamic: `minUsersPerTeam`, `maxUsersPerTeam`, `timeElapsedToUseMinimumUsers`src/types/queueDocument.ts (1)
2-10: Interface shape looks good; optional immutability nitsConsider marking fields as readonly to prevent accidental mutation in algorithms (they read but don’t write). Also, if ranked fields must satisfy rankedMin ≤ rankedValue ≤ rankedMax when present, document/enforce that at the schema layer.
-export interface QueueDocument { - _id: string; // Party ID - userIds: number[]; +export interface QueueDocument { + readonly _id: string; // Party ID + readonly userIds: readonly number[]; queueId: string; timeAdded: Date; // ranked specific rankedValue?: number; rankedMax?: number; rankedMin?: number; }AGENTS.md (1)
5-47: Docs read well; a couple of clarity/grammar nits
- Prefer “Type-check via tsc (noEmit)” to match TS flag casing.
- “types/interfaces PascalCase within code” → “Types and interfaces use PascalCase; variables use camelCase.”
- Consider adding a short note that Node support is experimental if Bun is the primary runtime (the code mixes both).
src/modules/matchmaking/algorithms/utils.ts (1)
18-44: Greedy packer works; remove redundant state and check fill immediately after placementplaced/continue are unnecessary, and checking “all filled” right after a successful placement avoids an extra pass each outer iteration.
export function packTeamsGreedy( parties: WithId<QueueDocument>[], teamsPerMatch: number, usersPerTeam: number ): { teams: number[][]; partiesUsed: string[] } | null { const sorted = [...parties].sort((a, b) => b.userIds.length - a.userIds.length); const teams = createTeamsArray(teamsPerMatch); const partiesUsed: string[] = []; for (const party of sorted) { - let placed = false; for (const team of teams) { if (team.length + party.userIds.length <= usersPerTeam) { team.push(...party.userIds); partiesUsed.push(party._id); - placed = true; + // Early success check after each placement + if (teams.every((t) => t.length === usersPerTeam)) { + return { teams, partiesUsed }; + } break; } } - const allFilled = teams.every((t) => t.length === usersPerTeam); - if (allFilled) return { teams, partiesUsed }; - if (!placed) continue; } // Not able to exactly fill all teams return null; }src/schemas/leaveQueue.ts (1)
4-5: Trim input to avoid whitespace-only IDsMinor hardening: trim the string before length validation.
-export const LeaveQueueSchema = z.object({ - partyId: z.string().min(1) -}); +export const LeaveQueueSchema = z.object({ + partyId: z.string().trim().min(1), +});src/index.ts (2)
26-29: Exiting the primary when a worker dies may cause avoidable downtimeprocess.exit() on any worker “exit” takes the whole app down. Prefer respawn-or-log, or rely on an external supervisor without killing the primary.
- cluster.on("exit", (worker, code, signal) => { - console.log(`Worker ${worker.process.pid} died`); - process.exit(); - }); + cluster.on("exit", (worker, code, signal) => { + console.warn(`Worker ${worker.process.pid} died (code=${code}, signal=${signal}); forking a replacement.`); + cluster.fork(); + });
11-14: Parse Instances with radix and clamp to a sane rangeAvoid parseInt pitfalls and negative/NaN values.
- let instances = numCPUs - 1; + let instances = Math.max(1, numCPUs - 1); if (process.env.Instances) { - instances = parseInt(process.env.Instances); + const n = Number.parseInt(process.env.Instances, 10); + if (Number.isFinite(n) && n > 0) instances = n; }src/routes/v1/leave-queue.ts (1)
26-41: Prefer try/catch with result inspection over chained then/catch for clarity and to surface deletion outcome.Also avoid “return await” here.
- return await queuesCollection - .deleteOne({ - _id: partyId - }) - .then(() => { - // success even if not found in queue so we can return a success status - return c.json({ - success: true, - status: "RemovedFromQueue" - }); - }) - .catch(() => { - return c.json({ - success: false - }); - }); + try { + const res = await queuesCollection.deleteOne({ _id: partyId /* + ownerId filter (see above) */ }); + // Intentionally returning success even if not found (idempotent) + return c.json({ success: true, status: "RemovedFromQueue", deletedCount: res?.deletedCount ?? 0 }); + } catch (err) { + // TODO: hook into logger if available + return c.json({ success: false }); + }src/modules/matchmaking/algorithms/index.ts (1)
7-11: Strongly type the registry per queueType to remove any-casts.Uses Extract to pick the right subtype from QueueConfig; safer and self-documenting.
-import type { QueueConfig } from "types/queues.js"; -import type { Algorithm } from "./types.js"; +import type { QueueConfig } from "types/queues.js"; +import type { Algorithm } from "./types.js"; import { findNormalMatch } from "./normal.js"; import { findDynamicMatch } from "./dynamic.js"; import { findRankedMatch } from "./ranked.js"; -export const algorithmRegistry: Record<QueueConfig["queueType"], Algorithm<any>> = { - normal: findNormalMatch as Algorithm<any>, - dynamic: findDynamicMatch as Algorithm<any>, - ranked: findRankedMatch as Algorithm<any> -}; +type Registry = { + [K in QueueConfig["queueType"]]: Algorithm<Extract<QueueConfig, { queueType: K }>>; +}; + +export const algorithmRegistry: Registry = { + normal: findNormalMatch, + dynamic: findDynamicMatch, + ranked: findRankedMatch +};src/modules/matchmaking/algorithms/types.ts (1)
7-13: Tighten Services typing for better contracts (optional).
- Give createMatch a concrete return type.
- Consider aliasing PartyId and Team as types to improve readability.
- Optional: log signature with level.
export type Services = { getOldestParties: (queueId: string, limit?: number) => Promise<WithId<QueueDocument>[]>; updatePartyRange?: (partyId: string, rankedMin: number, rankedMax: number) => Promise<void>; - createMatch: (queueData: QueueConfig, teams: number[][], partiesUsed: string[]) => Promise<any>; - now: () => Date; - log?: (message: string, meta?: Record<string, unknown>) => void; + createMatch: (queueData: QueueConfig, teams: number[][], partiesUsed: string[]) => Promise<{ matchId: string }>; + now: () => Date; + log?: (message: string, meta?: Record<string, unknown>, level?: "debug" | "info" | "warn" | "error") => void; };src/modules/matchmaking/algorithms/dynamic.ts (1)
12-13: Avoid magic number (2500) when fetching oldest parties.Scale the fetch limit from queue parameters or make it configurable.
- let allParties = await services.getOldestParties(queueId, 2500); + const fetchLimit = Math.max(teamsPerMatch * maxUsersPerTeam * 10, 250); + let allParties = await services.getOldestParties(queueId, fetchLimit);src/modules/matchmaking/algorithms/normal.ts (2)
24-27: Compute requiredPlayers once.It’s constant per run; avoid recomputing inside the loop.
- // Track used parties across iterations so we can keep forming matches + // Track used parties across iterations so we can keep forming matches const usedPartyIds = new Set<string>(); + const requiredPlayers = usersPerTeam * teamsPerMatch; ... - const requiredPlayers = usersPerTeam * teamsPerMatch;
21-22: Set + filtering are redundant.Since you already mutate allParties to exclude used, the usedPartyIds filter on availableParties is duplicative. Keep either the Set or the allParties pruning, not both.
Also applies to: 33-34
src/modules/matchmaking/matchmaking.ts (1)
11-20: Minor: project matchId only.Save bandwidth with a projection when fetching from foundPartiesCollection.
- const matchId: ObjectId | null = await foundPartiesCollection - .findOne({ _id: partyId }) + const matchId: ObjectId | null = await foundPartiesCollection + .findOne({ _id: partyId }, { projection: { matchId: 1 } })src/modules/matchmaking/algorithms/ranked.ts (2)
103-107: Compute requiredPlayers once.Minor micro-optimization and clarity.
-export const findRankedMatch: Algorithm<QueueConfig & { queueType: "ranked" }> = async (queueData, services) => { - const { queueId, usersPerTeam, teamsPerMatch, searchRange } = queueData; +export const findRankedMatch: Algorithm<QueueConfig & { queueType: "ranked" }> = async (queueData, services) => { + const { queueId, usersPerTeam, teamsPerMatch, searchRange } = queueData; + const requiredPlayers = usersPerTeam * teamsPerMatch; ... - if (totalPlayers >= usersPerTeam * teamsPerMatch) { + if (totalPlayers >= requiredPlayers) {
83-85: Unify default range logic.findRankedMatch defaults to rankedValue ± searchRange, expandSearchRange starts from rankedValue if min/max are null. Consider a single helper to compute effective range to avoid drift between code paths.
Also applies to: 16-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (39)
.prettierrc(1 hunks)AGENTS.md(1 hunks)README.md(1 hunks)docs/README.md(1 hunks)docs/configuration.md(1 hunks)docs/matchmaking.md(1 hunks)package.json(2 hunks)src/index.ts(3 hunks)src/middlewares/body-parser.ts(1 hunks)src/middlewares/error-handler.ts(1 hunks)src/middlewares/logging.ts(1 hunks)src/middlewares/parse-gzip-json.ts(1 hunks)src/modules/authorization.ts(1 hunks)src/modules/config.ts(1 hunks)src/modules/database-indexes.ts(1 hunks)src/modules/database.ts(1 hunks)src/modules/empty-handler.ts(1 hunks)src/modules/matchmaking/algorithms/dynamic.ts(1 hunks)src/modules/matchmaking/algorithms/index.ts(1 hunks)src/modules/matchmaking/algorithms/normal.ts(1 hunks)src/modules/matchmaking/algorithms/ranked.ts(1 hunks)src/modules/matchmaking/algorithms/types.ts(1 hunks)src/modules/matchmaking/algorithms/utils.ts(1 hunks)src/modules/matchmaking/matchmaking.ts(1 hunks)src/routes/v1/healthcheck.ts(1 hunks)src/routes/v1/join-queue.ts(1 hunks)src/routes/v1/leave-queue.ts(1 hunks)src/schemas/joinQueue.ts(1 hunks)src/schemas/leaveQueue.ts(1 hunks)src/schemas/validQueues.ts(1 hunks)src/types/foundMatchDocument.ts(1 hunks)src/types/foundPartyDocument.ts(1 hunks)src/types/queueDocument.ts(1 hunks)src/types/queues.ts(1 hunks)src/types/serverDocument.ts(1 hunks)tests/configuration.ts(1 hunks)tests/manual/join-queue.ts(1 hunks)tests/manual/leave-queue.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[grammar] ~12-~12: There might be a mistake here.
Context: ... start: Run the server once (uses Bun). - bun run build: Type-check via tsc` (no emit). - Exam...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...alCase within code; variables camelCase. - Validation: zod schemas in src/schemas...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...nt, Instances, MATCHMAKING_ENABLED`. - Start MongoDB locally and ensure indexes...
(QB_NEW_EN)
docs/matchmaking.md
[grammar] ~17-~17: There might be a mistake here.
Context: ...ce and Registry - Algorithm interface: Algorithm<T extends QueueConfig> = (cfg: T, services: Services) => Promise<void> - Services provide DB access and utilities...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...disables sharding (global matching only) - >= 0: match within shard until the oldest pa...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...msPerMatch, discoverMatchesInterval, shardTimeoutSeconds- Ranked:searchRange, incrementRange`,...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...anked: searchRange, incrementRange, incrementRangeMax? - Dynamic: minUsersPerTeam, `maxUsersPer...
(QB_NEW_EN)
🔇 Additional comments (14)
src/schemas/validQueues.ts (1)
5-5: No functional change — looks good.This is a formatting-only diff; exports remain unchanged.
README.md (1)
38-41: LGTM — spacing improves readability before the list.docs/configuration.md (1)
69-71: Formatting aligns with repository Prettier rules.src/modules/database-indexes.ts (3)
20-22: TTL index usage: double-check field types are Date.Ensure timeAdded/createdAt are Date objects to avoid TTL no-op. If any are numbers/strings, documents won’t expire.
26-28: LGTM.Consistent with other TTLs; naming consistent.
32-35: LGTM.Same pattern; no issues spotted.
tsconfig.json (1)
2-20: Confirm intent: noEmit true with a “build”: “tsc” script produces no files.If you expect dist artifacts, set noEmit to false (or remove it) and keep outDir; otherwise, this is fine for type-check only.
src/types/serverDocument.ts (1)
2-4: LGTM—style normalization only.No type/signature changes.
src/modules/config.ts (1)
20-31: Verify queue search params are intentional.
searchRange: [0,0]withincrementRange: [1,1]means no initial search breadth; ensure the discovery loop expands as expected.Would you like me to scan the algorithm code to confirm it widens the range over time?
src/types/foundPartyDocument.ts (1)
4-7: LGTMFormatting-only; types remain unchanged.
src/middlewares/parse-gzip-json.ts (1)
18-19: Always calling next() is correctMiddleware now consistently advances the chain. Good.
docs/README.md (1)
6-6: LGTMGood addition and placement.
src/modules/matchmaking/algorithms/utils.ts (1)
4-14: LGTM on helperscreateTeamsArray, sumUsers, and ageSeconds are clean and correct.
src/routes/v1/leave-queue.ts (1)
27-29: _id type confirmed as string—no changes required.
QueueDocument._id is declared as string in src/types/queueDocument.ts, so deleteOne({_id: partyId}) works as-is.
| .then((newDoc) => { | ||
| return { | ||
| success: true, | ||
| status: "InQueue", | ||
| queueData: newDoc | ||
| }; | ||
| }) | ||
| .catch(() => { | ||
| return { | ||
| success: false | ||
| }; | ||
| }); | ||
|
|
||
| return c.json(result); | ||
| }); | ||
| return c.json(result); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return proper HTTP status on DB failure
Currently returns 200 with { success: false }. Respond 500 to signal failure.
- .catch(() => {
- return {
- success: false
- };
- });
+ .catch(() => {
+ c.status(500);
+ return { success: false, error: "Failed to join queue" };
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .then((newDoc) => { | |
| return { | |
| success: true, | |
| status: "InQueue", | |
| queueData: newDoc | |
| }; | |
| }) | |
| .catch(() => { | |
| return { | |
| success: false | |
| }; | |
| }); | |
| return c.json(result); | |
| }); | |
| return c.json(result); | |
| .then((newDoc) => { | |
| return { | |
| success: true, | |
| status: "InQueue", | |
| queueData: newDoc | |
| }; | |
| }) | |
| .catch(() => { | |
| c.status(500); | |
| return { success: false, error: "Failed to join queue" }; | |
| }); | |
| return c.json(result); |
🤖 Prompt for AI Agents
In src/routes/v1/join-queue.ts around lines 74 to 87, the DB failure branch
currently resolves to { success: false } and the handler always returns 200;
change the catch path to surface a 500 HTTP status and an error payload (e.g.,
set result to include success: false and an error message) and then return the
response with c.status(500).json(result) instead of c.json(result) so DB
failures are signaled with HTTP 500.
| const { partyId } = c.bodyData; | ||
|
|
There was a problem hiding this comment.
Enforce ownership check before deleting a party from the queue (authorization gap).
Currently any authenticated caller can supply an arbitrary partyId and remove it. Filter deleteOne (and any pre-checks) by the requesting user’s identifier (e.g., leader/owner). Adapt field names to your data model.
- return await queuesCollection
- .deleteOne({
- _id: partyId
- })
+ // TODO: adjust `ownerId`/context getter to your auth model
+ const userId = c.get?.("userId") ?? c.var?.userId;
+ return await queuesCollection
+ .deleteOne({
+ _id: partyId,
+ ownerId: userId
+ })
.then(() => {Also applies to: 26-35
🤖 Prompt for AI Agents
In src/routes/v1/leave-queue.ts around lines 15-16 and also 26-35, the handler
reads partyId from the body but does not verify the requesting user owns or
leads the party before deleting it. Change the logic to obtain the requester
identifier from the request context (e.g., c.user.id or c.auth.userId), then
either (a) fetch the party by partyId and check that its leader/owner field
matches the requester and return 403 if not, or (b) include the owner/leader
field equal to the requester in the deleteOne filter so the delete only succeeds
for parties owned by the caller; adapt the leader/owner field name to match your
data model and ensure a clear 403 error response when unauthorized.
| timeElaspedToUseMinimumUsers: number; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix typo in config property: timeElaspedToUseMinimumUsers → timeElapsedToUseMinimumUsers (breaking change).
Rename the field and update all references; consider a temporary compatibility fallback in code and a data migration if persisted.
- timeElaspedToUseMinimumUsers: number;
+ timeElapsedToUseMinimumUsers: number;Follow-ups outside this file (apply across codebase):
- Update dynamic algorithm destructure/uses.
- Migrate stored queue configs or handle both keys during a transition.
To find all occurrences:
I can draft a small migration or a runtime shim if you prefer.
🏁 Script executed:
#!/bin/bash
rg -n 'timeElaspedToUseMinimumUsers' -SLength of output: 527
Rename timeElaspedToUseMinimumUsers → timeElapsedToUseMinimumUsers
Update all occurrences across code and docs:
- src/types/queues.ts (property definition)
- src/modules/matchmaking/algorithms/dynamic.ts (destructure & conditional)
- docs/matchmaking.md (documentation)
Consider a backward-compatibility shim or a data migration for any persisted configs.
- timeElaspedToUseMinimumUsers: number;
+ timeElapsedToUseMinimumUsers: number;🤖 Prompt for AI Agents
In src/types/queues.ts around lines 19-20, rename the exported property
timeElaspedToUseMinimumUsers to timeElapsedToUseMinimumUsers and update all
references across the codebase and docs (notably
src/modules/matchmaking/algorithms/dynamic.ts where it’s destructured and used
in conditionals, and docs/matchmaking.md). Replace the misspelled identifier
everywhere (types, imports, destructures, property access) and run type checks
to catch remaining references; add a backward-compatibility shim or migration:
accept the old key (timeElaspedToUseMinimumUsers) when reading persisted configs
and map it to the new key, and update any persisted schemas or docs accordingly.
| import { randomUUID } from "node:crypto"; | ||
|
|
||
| const queueId = prompt("Queue ID:") | ||
| const queueId = prompt("Queue ID:"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm runtime: prompt in Node will throw.
Switch to readline/promises or CLI args if these scripts run under Node.
Also applies to: 6-7, 9-9
🏁 Script executed:
#!/bin/bash
rg -nP '\bprompt\s*\(' tests/manual -S -C2Length of output: 905
Avoid prompt in Node; replace with readline/promises or CLI args
- tests/manual/join-queue.ts: lines 4, 6, 7, 9
- tests/manual/leave-queue.ts: line 4
prompt will throw under Node—use readline/promises or parse process.argv instead.
🤖 Prompt for AI Agents
In tests/manual/join-queue.ts (line 4) and tests/manual/leave-queue.ts (line 4),
using browser prompt() will throw in Node; replace it with either Node's
readline/promises or by reading CLI args from process.argv. Update the file(s)
to: import readline/promises (or use process.argv), create a readline interface
and await question("Queue ID:") to capture input (then close the interface), or
parse a --queue-id flag from process.argv and validate it; remove any uses of
prompt() and ensure the new input method is asynchronous where needed and
handles missing/invalid values with a clear error message and process.exit.
| import { randomUUID } from "node:crypto"; | ||
|
|
||
| const partyId = prompt("Party ID:") | ||
| const partyId = prompt("Party ID:"); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm runtime: prompt isn’t available in Node.
If you run this with Node, prompt will throw. Consider switching to readline/promises or pass IDs via CLI args/env.
Run to locate all usages:
🏁 Script executed:
#!/bin/bash
rg -nP '\bprompt\s*\(' -g '!**/node_modules/**' -SLength of output: 494
Replace browser prompt calls in manual test scripts with Node-compatible input
- In tests/manual/leave-queue.ts (line 4) and tests/manual/join-queue.ts (lines 4, 6, 7, 9),
prompt(…)isn’t provided by Node and will throw at runtime. - Use Node’s
readline/promisesAPI or accept IDs via CLI arguments or environment variables instead.
🤖 Prompt for AI Agents
In tests/manual/leave-queue.ts around line 4, replace the browser-only
prompt("Party ID:") with a Node-compatible input method: either read the party
ID from process.argv or process.env, or use the Node readline/promises API to
asynchronously prompt the user; update the code to import and use
readline/promises (or parse CLI args) and ensure the input is validated and
awaited before continuing.
Summary by CodeRabbit