More Biome rules! #200
More Biome rules! #200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request introduces pathfinding and search features with new services, utilities, and controllers. It adds comprehensive development tooling including markdownlint, Biome rule validation, and enhanced CI checks. Multiple controllers are updated with explicit public modifiers and WebSocket integration, types are extended to support navigation instructions, and frontend components are updated to use new instruction field naming. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PathController
participant PathService
participant Pathfinder
participant Database
Client->>PathController: GET /path?start=X&end=Y<br/>(BEARER_AUTH)
PathController->>PathService: calculatePath(start, end)
PathService->>Pathfinder: parseWaypoint(start)<br/>parseWaypoint(end)
Pathfinder->>Database: (async) graph lookup
Database-->>Pathfinder: nodes, edges, buildings
Pathfinder->>Pathfinder: waypointToNodes()<br/>convertToGraphNodes
Pathfinder->>Pathfinder: findPath()<br/>Dijkstra search<br/>with outside penalty
Pathfinder->>Pathfinder: getPreciseRoute()<br/>generateInstructions()
Pathfinder-->>PathService: PreciseRoute
PathService-->>PathController: Record<string, PreciseRoute>
PathController-->>Client: 200 OK + JSON
sequenceDiagram
participant Client
participant SearchController
participant SearchService
participant SearchUtils
participant Database
Client->>SearchController: GET /search?query=Q&n=N&lat=L&lon=LO
SearchController->>SearchService: search(query, n, lat, lon)
SearchService->>SearchUtils: parseQuery(query)<br/>→ trigrams
SearchService->>SearchUtils: getSearchContext()<br/>(lazy init + cache)
SearchUtils->>Database: fetch buildings, floorplans
Database-->>SearchUtils: raw data
SearchUtils->>SearchUtils: buildSearchIndex()<br/>buildingToDocument()<br/>roomToDocument()<br/>insertTerms()
SearchUtils-->>SearchService: index, documents
SearchService->>SearchService: match trigrams<br/>compute BM25Term scores<br/>per-document accumulation
alt lat/lon provided
SearchService->>SearchService: distanceWeightedScore()<br/>boost room docs
end
SearchService->>SearchService: topN() sort results
SearchService-->>SearchController: Document[]
SearchController-->>Client: 200 OK + JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| @Body() body: { poiInfo: PoiInfo }, | ||
| ) { | ||
| const poiId = req.params.poiId; | ||
| const { poiId, socketId } = req.params; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Yuxiang-Huang
force-pushed
the
biome-rules
branch
from
ee579f5 to
0f76e2b
Compare
Yuxiang-Huang
force-pushed
the
biome-rules
branch
from
064e127 to
9951294
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/server/src/services/floorService.ts (1)
333-333: Typo in comment.- // Convert thGeoNodeses to the format expected by the frontend + // Convert the GeoNodes to the format expected by the frontend.github/workflows/check.yml (1)
71-71: Fix typo in comment.The comment contains a typo: "generaet" should be "generate".
🔎 Proposed fix
- - name: Install dependencies, generaet DB schema and OpenAPI spec + - name: Install dependencies, generate DB schema and OpenAPI spec
🤖 Fix all issues with AI agents
In @apps/server/Dockerfile:
- Line 40: The Dockerfile currently copies Prisma engines to /tmp/prisma-engines
(COPY --from=builder --chown=server:nodejs /app/apps/server/node_modules/prisma
/tmp/prisma-engines) but PrismaClient is not configured to look there; either
set the PRISMA_QUERY_ENGINE_BINARY env var to point at the engine binary under
/tmp/prisma-engines (add an ENV PRISMA_QUERY_ENGINE_BINARY=... line) so
PrismaClient finds the engine, or instead copy the full node_modules for Prisma
into the runtime image so engines remain in their default location; update the
Dockerfile to perform one of these fixes and ensure PrismaClient will locate the
binary at runtime.
In @apps/server/src/controllers/nodeController.ts:
- Line 30: The handlers createNode, deleteNode, and updateNode in
nodeController.ts are currently destructuring socketId from req.params but the
requireSocketId middleware attaches it as req.socketId; change each handler to
stop reading socketId from req.params and instead read socketId from
req.socketId (and update any typings or interfaces for the request object if
necessary) so broadcasts use the correct socketId value.
In @apps/server/src/controllers/poiController.ts:
- Line 27: The handler incorrectly reads socketId from req.params; change the
extraction so poiId still comes from req.params (e.g., keep const { poiId } =
req.params) and obtain socketId from req.socketId (set by requireSocketId
middleware) before using it for broadcasts; update any references that assume
socketId came from params to use the new socketId variable so WebSocket
broadcasts receive the correct id.
- Line 38: In the POI controller handler replace destructuring that reads
socketId from req.params (const { poiId, socketId } = req.params) so that poiId
still comes from req.params but socketId is read from req.socketId; update the
code paths in the same function (the handler in poiController.ts) to use the new
socketId source and remove reliance on params.socketId.
- Line 50: The destructuring currently pulls socketId from req.params which is
incorrect; in poiController.ts keep poiId from req.params (const { poiId } =
req.params) and obtain socketId from the query (const { socketId } = req.query)
or wherever the rest of the app supplies it (e.g., req.body) so the socketId
variable is read from the correct source before use in the controller.
In @apps/server/src/controllers/searchController.ts:
- Around line 8-15: The search controller's search method lacks input validation
for query, n, lat, and lon; add validation at the start of the public async
search(...) in searchController.search: ensure query is non-empty (throw
BadRequestException or similar), set a sane default for n (e.g., 10) and enforce
a max (e.g., <=100) and non-negative, and validate lat is between -90 and 90 and
lon between -180 and 180 (throw BadRequestException with clear messages on
invalid values); after these checks call searchService.search(query, n, lat,
lon) with the validated/normalized values.
In @apps/server/src/utils/path/pathfinder.ts:
- Around line 115-132: The code currently mutates the passed-in graph by adding
dummyNode and updating neighbor.neighbors; change this to non-mutating behavior
by operating on a shallow copy: create newGraph = { ...graph }, for each
neighborId clone the neighbor object and its neighbors map (e.g., neighborClone
= { ...neighbor, neighbors: { ...neighbor.neighbors } }) before assigning
neighborClone.neighbors[dummyNode.id] = {} and inserting dummyNode into newGraph
(instead of graph); then return the newGraph (or return the created dummyNode
separately) so callers receive the updated structure without side effects.
In @packages/common/src/utils/coordinates.ts:
- Around line 166-179: The function geoNodeToNavPathNode currently maps
geoNode.roomId to the literal "outside"; change this so NavPathNode preserves
the semantic absence of a room by either setting roomId to geoNode.roomId ??
null or making NavPathNode.roomId optional and assigning roomId: geoNode.roomId,
and update the NavPathNode type definition to accept null (string | null) or
undefined accordingly; modify any callers that assume the "outside" string to
handle null/undefined instead.
In @scripts/biome/check-biome-config.ts:
- Around line 1-15: Wrap the async operations in check-biome-config.ts (the
await getAllRules(), await fs.readFile(BIOME_CONFIG_PATH, ...) and
JSON.parse(...)) in a try/catch so failures are caught; on error call
processLogger or console.error with the caught error (include message/stack) and
process.exit(1). Specifically, surround the logic that uses getAllRules,
fs.readFile and JSON.parse, validate biomeConfig.linter and rules exist before
comparing, and ensure any thrown error is logged and leads to a non-zero exit
instead of an unhandled rejection.
- Line 8: Replace the raw JSON.parse call on the BIOME_CONFIG_PATH file with a
JSONC-aware parse (e.g., use a jsonc parser or strip comments before parsing) so
files with comments (biome.jsonc) don't crash; after parsing into biomeConfig,
defensively check that biomeConfig.linter and biomeConfig.linter.rules exist and
are the expected types before accessing them (refactor any direct accesses to
biomeConfig.linter.rules into guarded checks and throw or log a clear error if
the structure is missing).
In @scripts/biome/write-biome-config.ts:
- Around line 1-10: The script lacks error handling around async/network and
file operations; wrap the main sequence (await getAllRules(),
fs.readFile(BIOME_CONFIG_PATH), JSON.parse, and fs.writeFile) in a try/catch,
validate that the parsed object has a biomeConfig.linter object before assigning
biomeConfig.linter.rules = allRules (create it if missing), and on error log a
clear message including the caught error via console.error or
processLogger.error and terminate with process.exit(1); also consider catching
specific JSON.parse failures to report malformed config and rethrow or exit
appropriately.
- Around line 5-6: The code uses JSON.parse on the file read at
BIOME_CONFIG_PATH which may be JSONC (comments) and also assumes
biomeConfig.linter exists; replace the parse with a JSONC-aware parse (or strip
comments before parsing) when reading via fs.readFile, and ensure you
validate/create biomeConfig.linter before assigning biomeConfig.linter.rules =
allRules (e.g., check biomeConfig && typeof biomeConfig === "object" and if
(!biomeConfig.linter) set biomeConfig.linter = {}), keeping references to
BIOME_CONFIG_PATH, biomeConfig, allRules, and the fs.readFile usage so the
change is made where the file is read and modified.
🧹 Nitpick comments (20)
apps/server/src/controllers/edgeController.ts (1)
76-81: LGTM: Clean refactoring reduces nesting.Consolidating the nested
else { if (...) }intoelse if (...)improves code readability without changing the logic. This is a good code quality improvement..markdownlint-cli2.jsonc (1)
21-22: Minor: Comment is slightly misleading.The comment says "Disable sibling list check" but
{ "siblings_only": true }doesn't disable MD024—it allows duplicate headings in different sections while still flagging duplicates among sibling headings. Consider updating the comment to accurately describe the behavior, e.g., "Allow duplicate headings in different sections".packages/common/src/types/graphTypes.ts (1)
56-64: Consider:roomIdtype inconsistency withGeoNode.
NavPathNode.roomIdis typed asstring(required), whileGeoNode.roomIdat line 51 isstring | null. If a navigation path node can exist without an associated room, consider aligning the types:export interface NavPathNode { neighbors: { [neighborId: string]: EdgeInfo; }; - roomId: string; + roomId: string | null; floor?: Floor; coordinate: GeoCoordinate; id: string; }scripts/biome/check-biome-config.ts (1)
10-10: Consider usingconsole.loginstead ofconsole.debugfor CI visibility.
console.debugmay not be visible in CI environments depending on log level configuration. Since this script is meant to run in CI (as indicated by the check workflow), usingconsole.logensures the success message is always visible.- console.debug("Biome config is up to date"); + console.log("✓ Biome config is up to date");.husky/pre-commit (1)
3-3: Consider the implications of sourcing user shell config.Sourcing
~/.zshrcin a git hook can cause issues if the file doesn't exist, contains interactive commands, or has side effects that interfere with the hook's execution. Git hooks typically run in a minimal environment.If this is only needed for bun/bunx to be in PATH, consider sourcing a minimal environment file or using absolute paths to the executables instead.
apps/web/src/components/info-cards/nav-card-desktop/NavCardDesktop.tsx (1)
179-181: Remove debug console.log statement.This appears to be a leftover debug statement that should be removed before merging.
🔎 Proposed fix
- useEffect(() => { - console.log("rendering directions list"); - }, []);apps/server/src/utils/path/instructions.ts (2)
4-4: Placeholder constant suggests incomplete implementation.The magic number
42forPLACEHOLDER_INSTRUCTION_DISTANCEindicates this is a temporary value. Consider adding a TODO comment or tracking this for future implementation of actual distance calculation.Would you like me to open an issue to track the implementation of actual distance calculations for navigation instructions?
6-27: Consider adding "Arrive" instruction for the destination node.The function only generates turn instructions for intermediate nodes but doesn't include arrival at the destination. Navigation flows typically need an "Arrive" instruction for the final node.
🔎 Proposed enhancement
for (let i = 0; i < path.length - 2; i++) { // ... existing turn detection logic } + // Add arrival instruction for the final node + if (path.length > 0) { + const lastNode = path[path.length - 1]; + instructions.push({ + action: "Arrive", + distance: 0, + nodeId: lastNode.id, + }); + } + return instructions; }apps/server/src/services/pathService.ts (2)
91-93: Consider caching the buildings query.
prisma.building.findMany()is called on every path calculation but buildings data rarely changes. Consider caching it alongside the graph for better performance.🔎 Proposed approach
let graphCache: GeoNodes | null = null; +let buildingsCache: Buildings | null = null; async function getOrBuildGraph(): Promise<GeoNodes> { // ... existing code } +async function getOrFetchBuildings(): Promise<Buildings> { + if (!buildingsCache) { + buildingsCache = await prisma.building.findMany(); + } + return buildingsCache; +} export const pathService = { async calculatePath(/* ... */): Promise<Record<string, PreciseRoute>> { // ... const graph = await getOrBuildGraph(); - const buildings = await prisma.building.findMany(); + const buildings = await getOrFetchBuildings(); // ... }, };
14-16: Potential race condition during concurrent graph initialization.If multiple requests arrive before the graph is cached, they could all trigger concurrent graph builds. Consider adding a mutex or using a promise-based lock pattern.
🔎 Proposed approach using promise caching
-let graphCache: GeoNodes | null = null; +let graphCachePromise: Promise<GeoNodes> | null = null; async function getOrBuildGraph(): Promise<GeoNodes> { - if (!graphCache) { + if (!graphCachePromise) { + graphCachePromise = buildGraph(); + } + return graphCachePromise; +} + +async function buildGraph(): Promise<GeoNodes> { console.log("Building graph cache..."); // ... rest of graph building logic - graphCache = graph; - } - return graphCache; + return graph; }apps/server/src/utils/search/rank.ts (1)
62-67: Mutation of input array whenrankList.length <= n.When
sort=trueand the list is small,result.sort()mutates the originalrankListarray sinceresultis a direct reference to it. Consider cloning to avoid side effects.🔎 Proposed fix to avoid mutation
if (rankList.length <= n) { - result = rankList; - if (sort) result.sort((a, b) => b[1] - a[1]); + result = sort + ? [...rankList].sort((a, b) => b[1] - a[1]) + : rankList; } else { - result = rankList.sort((a, b) => b[1] - a[1]).slice(0, n); + result = [...rankList].sort((a, b) => b[1] - a[1]).slice(0, n); }scripts/biome/generate-biome-rules.ts (2)
97-122: Missing error handling for external schema fetch.The
getAllRulesfunction fetches from an external URL without error handling. If the fetch fails or returns non-JSON, the script will crash with an unclear error.🔎 Proposed fix to add error handling
export async function getAllRules() { + const response = await fetch("https://biomejs.dev/schemas/latest/schema.json"); + if (!response.ok) { + throw new Error(`Failed to fetch Biome schema: ${response.status} ${response.statusText}`); + } const schema = (await ( - await fetch("https://biomejs.dev/schemas/latest/schema.json") + response ).json()) as BiomeSchema;
137-141: TypeAllRulesdoesn't match actual rule value shapes.The
AllRulestype defines rule values asstring, but OVERRIDES includes object values withlevelandoptionsproperties (e.g., lines 74-79, 82-87). This type mismatch could cause type errors or confusion.🔎 Proposed fix to align the type with actual usage
+type RuleValue = string | { level: string; options: Record<string, unknown> }; + interface AllRules { [groupName: string]: { - [ruleName: string]: string; + [ruleName: string]: RuleValue; }; }apps/server/src/services/searchService.ts (1)
69-96: Verbose per-term logging may impact performance and clutter logs.Logging for each processed query term (line 95) will generate significant log output for queries with many terms. Consider reducing log verbosity or using a debug flag.
🔎 Proposed fix to reduce logging verbosity
- for (const word of queryTerms) { + for (const [i, word] of queryTerms.entries()) { const relDocs = index[word]; if (!relDocs || relDocs.length === 0) { continue; } const docFreq = relDocs.length; // Score each document containing this term for (const [docId, termFreq] of relDocs) { const doc = documents[docId]; if (!doc) continue; const docLen = doc.numTerms; const score = BM25Term(termFreq, docLen, docFreq, numDocs, avgDl); // Accumulate scores const currentScore = overallScores.get(docId) ?? 0; overallScores.set(docId, currentScore + score); } - - console.log(`Processed word '${word}' at ${Date.now() - startTime}ms`); } + + console.log(`Processed ${queryTerms.length} terms at ${Date.now() - startTime}ms`);apps/server/src/utils/search/types.ts (1)
1-3: Move import statement before exports for conventional ordering.The
SearchIndextype export appears before the import statement, which is unconventional. While valid TypeScript, placing imports at the top improves readability.🔎 Proposed fix
+import type { Floor, GeoCoordinate, RoomType } from "@cmumaps/common"; + export type SearchIndex = Record<string, [string, number][]>; -import type { Floor, GeoCoordinate, RoomType } from "@cmumaps/common"; - export type Document = {apps/server/src/utils/path/pathfinder.ts (3)
48-56: Magic number 36 for UUID detection; consider a more explicit check.Using
s.length === 36to detect UUIDs is fragile. A UUID-4 pattern check would be more reliable, and the fallback to treat unknowns as room IDs (line 55) may cause unexpected behavior.🔎 Proposed fix with UUID regex
+const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export const parseWaypoint = (s: string): WayPoint => { if (s.includes(",")) { const [latStr, lonStr] = s.split(","); const latitude = Number(latStr); const longitude = Number(lonStr); if (Number.isFinite(latitude) && Number.isFinite(longitude)) { return { type: "Coordinate", coordinate: { latitude, longitude } }; } } if (s.length < 5) { return { type: "Building", buildingCode: s }; } - if (s.length === 36 && !s.includes(",")) { + if (UUID_REGEX.test(s)) { return { type: "Room", roomId: s }; } // For now, treat unknown as room return { type: "Room", roomId: s }; };
73-88: Inconsistent variable naming: use camelCase for consistency.Variables
best_distandbest_nodeuse snake_case, while the rest of the codebase uses camelCase.🔎 Proposed fix
case "Coordinate": { const coord = waypoint.coordinate; - let best_dist = null; - let best_node = null; + let bestDist: number | null = null; + let bestNode: string | null = null; for (const node of Object.values(graph)) { const d = dist(node.pos, coord); - if (best_dist === null || d < best_dist) { - best_dist = d; - best_node = node.id; + if (bestDist === null || d < bestDist) { + bestDist = d; + bestNode = node.id; } } - if (best_node === null) { + if (bestNode === null) { throw new Error("Graph should not be empty!"); } - nodes.push(best_node); + nodes.push(bestNode); break; }
246-253: Storing full path copies in each queue item may cause high memory usage.Each queue item stores a copy of the entire path so far (
[...curPath, neighborId]). For large graphs with long paths, this can lead to significant memory consumption. Consider using a predecessor map and reconstructing the path at the end.🔎 Proposed optimization using predecessor tracking
export const findPath = ( startNodes: string[], endNodes: string[], graph: GeoNodes, outsideCostMul = 1, ): Route | null => { - type QueueItem = { path: string[]; distance: number; addCost: number }; + type QueueItem = { nodeId: string; distance: number; addCost: number }; const pq = new TinyQueue<QueueItem>([], (a, b) => a.distance - b.distance); const exploredSet = new Set<string>(); + const predecessor = new Map<string, string>(); + const distanceMap = new Map<string, number>(); + const addCostMap = new Map<string, number>(); const isGoal = new Set(endNodes); // Push all start nodes for (const nodeId of startNodes) { const node = graph[nodeId]; if (!node) continue; - pq.push({ path: [node.id], distance: 0, addCost: 0 }); + pq.push({ nodeId: node.id, distance: 0, addCost: 0 }); + distanceMap.set(node.id, 0); + addCostMap.set(node.id, 0); } while (pq.length) { const current = pq.pop(); if (!current) break; - const curPath = current.path; - const curAddCost = current.addCost; - const pathLen = curPath.length; - const lastNodeId = curPath[pathLen - 1]; + const lastNodeId = current.nodeId; + const curAddCost = current.addCost; const lastNode = graph[lastNodeId]; if (!lastNode) continue; if (!exploredSet.has(lastNodeId)) { exploredSet.add(lastNodeId); } else { continue; } if (isGoal.has(lastNodeId)) { + // Reconstruct path + const path: string[] = []; + let curr: string | undefined = lastNodeId; + while (curr !== undefined) { + path.unshift(curr); + curr = predecessor.get(curr); + } return { distance: current.distance, path: { - path: curPath, + path, addCost: curAddCost.toString(), } as GraphPath, } as Route; } // ... rest of neighbor processing with predecessor tracking }apps/server/src/utils/search/build.ts (2)
27-36: Duplicate term extraction: parsing same values multiple times.
doc.aliasequalscode, anddoc.nameWithSpace/doc.fullNameWithSpaceequalname. The code parses these duplicate values, inflatingnumTermsand the index with redundant entries.🔎 Proposed fix to deduplicate terms
export function buildingToDocument({ id, name, code, labelPosition, }: { id: string; name: string; code: string; labelPosition: GeoCoordinate; }): { doc: Document; terms: string[] } { const doc: Document = { id, nameWithSpace: name, fullNameWithSpace: name, labelPosition: labelPosition, type: "building", alias: code, numTerms: 0, }; - const terms = [ - ...parseQuery(name), - ...parseQuery(code), - ...parseQuery(id), - ...parseQuery(doc.alias), - ...parseQuery(doc.nameWithSpace), - ...parseQuery(doc.fullNameWithSpace), - ]; + // Deduplicate source values before parsing + const uniqueSources = new Set([name, code, id]); + const terms = [...uniqueSources].flatMap((src) => parseQuery(src)); doc.numTerms = terms.length; return { doc, terms }; }
63-72: Same duplicate term issue inroomToDocument.Similar to
buildingToDocument, this function parses overlapping values (e.g.,aliasparsed twice,doc.nameWithSpacederived frombuildingCode + name).🔎 Proposed fix
- const terms = [ - ...parseQuery(name), - ...parseQuery(alias), - ...parseQuery(id), - ...parseQuery(doc.alias), - ...parseQuery(doc.nameWithSpace), - ...parseQuery(doc.fullNameWithSpace), - ]; + // Include composite names for better matching, deduplicate base values + const sources = new Set([name, alias, id, doc.nameWithSpace, doc.fullNameWithSpace]); + const terms = [...sources].flatMap((src) => parseQuery(src)); doc.numTerms = terms.length;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.devcontainer/devcontainer.json.github/CONTRIBUTING.md.github/workflows/check.yml.husky/pre-commit.markdownlint-cli2.jsonc.markdownlint.jsoncapps/server/Dockerfileapps/server/package.jsonapps/server/src/controllers/buildingController.tsapps/server/src/controllers/dropTablesController.tsapps/server/src/controllers/edgeController.tsapps/server/src/controllers/floorController.tsapps/server/src/controllers/nodeController.tsapps/server/src/controllers/pathController.tsapps/server/src/controllers/poiController.tsapps/server/src/controllers/populateTableController.tsapps/server/src/controllers/searchController.tsapps/server/src/services/floorService.tsapps/server/src/services/pathService.tsapps/server/src/services/searchService.tsapps/server/src/utils/path/instructions.tsapps/server/src/utils/path/pathfinder.tsapps/server/src/utils/search/build.tsapps/server/src/utils/search/parse.tsapps/server/src/utils/search/rank.tsapps/server/src/utils/search/types.tsapps/visualizer/src/components/map-view/PlacementPanel.tsxapps/visualizer/src/components/shared/ErrorDisplay.tsxapps/visualizer/src/components/ui-layout/FloorSwitcher.tsxapps/visualizer/src/components/ui-layout/LoadingText.tsxapps/visualizer/src/components/ui-layout/MainDisplay.tsxapps/visualizer/src/components/ui-layout/NavBar.tsxapps/web/src/components/floor-switcher/FloorSwitcher.tsxapps/web/src/components/floor-switcher/mobile/FloorSwitcherDisplayMobile.tsxapps/web/src/components/info-cards/nav-card-desktop/NavCardDesktop.tsxapps/web/src/components/nav/NavCard.tsxapps/web/src/components/nav/NavDirectionsList.tsxapps/web/src/components/nav/NavLine.tsxapps/web/src/components/nav/NavOverlay.tsxapps/web/src/components/toolbar/SearchResults.tsxapps/web/src/hooks/useLocationParams.tsapps/web/src/hooks/useMapRegionChange.tsapps/web/src/hooks/useNavigationParams.tsapps/web/src/types/navTypes.tsapps/web/src/utils/floorUtils.tsapps/web/src/utils/geometry.tsbiome.jsonbiome.jsoncpackage.jsonpackages/common/src/types/floorTypes.tspackages/common/src/types/graphTypes.tspackages/common/src/types/index.tspackages/common/src/types/waypoint.tspackages/common/src/utils/coordinates.tsscripts/biome/check-biome-config.tsscripts/biome/generate-biome-rules.tsscripts/biome/write-biome-config.tsscripts/secrets/config.shscripts/secrets/pull.shscripts/secrets/push.shsyncpack.config.ts
💤 Files with no reviewable changes (3)
- apps/web/src/utils/geometry.ts
- .markdownlint.jsonc
- biome.json
🧰 Additional context used
🧬 Code graph analysis (19)
packages/common/src/types/waypoint.ts (1)
packages/common/src/types/coordTypes.ts (1)
GeoCoordinate(1-4)
scripts/biome/write-biome-config.ts (1)
scripts/biome/generate-biome-rules.ts (2)
getAllRules(97-122)BIOME_CONFIG_PATH(13-13)
apps/web/src/hooks/useNavigationParams.ts (1)
apps/web/src/utils/floorUtils.ts (1)
buildFloorCode(35-41)
apps/server/src/controllers/pathController.ts (3)
packages/common/src/types/graphTypes.ts (2)
Route(79-84)PreciseRoute(111-116)apps/server/src/middleware/authentication.ts (1)
BEARER_AUTH(7-7)apps/server/src/services/pathService.ts (1)
pathService(75-110)
apps/server/src/controllers/edgeController.ts (3)
apps/server/src/middleware/socketAuth.ts (1)
requireSocketId(34-48)apps/server/src/middleware/authentication.ts (2)
BEARER_AUTH(7-7)MEMBER_SCOPE(8-8)packages/common/src/utils/floorCodeUtils.ts (2)
extractBuildingCode(7-9)extractFloorLevel(17-19)
apps/web/src/components/nav/NavOverlay.tsx (1)
apps/server/src/controllers/pathController.ts (1)
path(10-15)
apps/server/src/controllers/poiController.ts (5)
apps/server/src/middleware/socketAuth.ts (1)
requireSocketId(34-48)apps/server/src/middleware/authentication.ts (2)
BEARER_AUTH(7-7)MEMBER_SCOPE(8-8)packages/common/src/types/poiTypes.ts (2)
PoiInfo(10-20)PoiType(8-8)apps/server/src/services/poiService.ts (1)
poiService(5-28)apps/server/src/server.ts (1)
webSocketService(38-38)
apps/server/src/utils/path/instructions.ts (3)
packages/common/src/types/graphTypes.ts (2)
GeoNode(36-54)Instruction(89-96)apps/web/src/types/navTypes.ts (1)
Instruction(19-19)packages/common/src/utils/coordinates.ts (1)
calculateAngle(145-164)
apps/server/src/utils/path/pathfinder.ts (4)
packages/common/src/types/graphTypes.ts (4)
GeoNode(36-54)Route(79-84)GraphPath(69-74)PreciseRoute(111-116)packages/common/src/utils/coordinates.ts (2)
dist(124-134)geoNodeToNavPathNode(166-179)packages/common/src/types/waypoint.ts (1)
WayPoint(3-15)apps/server/src/services/pathService.ts (1)
Buildings(9-9)
apps/server/src/utils/search/rank.ts (3)
apps/server/src/utils/search/types.ts (1)
Document(5-13)packages/common/src/types/coordTypes.ts (1)
GeoCoordinate(1-4)packages/common/src/utils/coordinates.ts (1)
dist(124-134)
packages/common/src/utils/coordinates.ts (2)
packages/common/src/types/coordTypes.ts (1)
GeoCoordinate(1-4)packages/common/src/types/graphTypes.ts (2)
GeoNode(36-54)NavPathNode(56-64)
apps/server/src/services/searchService.ts (5)
apps/server/src/utils/search/build.ts (1)
getSearchContext(213-220)apps/server/src/utils/search/types.ts (1)
Document(5-13)apps/server/src/utils/search/parse.ts (1)
parseQuery(22-35)packages/common/src/types/coordTypes.ts (1)
GeoCoordinate(1-4)apps/server/src/utils/search/rank.ts (3)
BM25Term(13-30)distanceWeightedScore(36-47)topN(55-70)
scripts/biome/check-biome-config.ts (1)
scripts/biome/generate-biome-rules.ts (2)
getAllRules(97-122)BIOME_CONFIG_PATH(13-13)
apps/server/src/controllers/nodeController.ts (4)
apps/server/src/middleware/socketAuth.ts (1)
requireSocketId(34-48)apps/server/src/services/edgeService.ts (1)
edgeService(3-36)apps/server/src/services/nodeService.ts (1)
nodeService(9-48)apps/server/src/server.ts (1)
webSocketService(38-38)
apps/web/src/hooks/useLocationParams.ts (1)
apps/web/src/utils/floorUtils.ts (1)
buildFloorCode(35-41)
apps/server/src/controllers/floorController.ts (3)
apps/server/src/middleware/authentication.ts (2)
BEARER_AUTH(7-7)MEMBER_SCOPE(8-8)packages/common/src/types/graphTypes.ts (1)
Route(79-84)apps/server/src/services/floorService.ts (1)
floorService(24-365)
apps/web/src/types/navTypes.ts (1)
packages/common/src/types/graphTypes.ts (1)
Instruction(89-96)
apps/server/src/controllers/populateTableController.ts (3)
apps/server/src/controllers/floorController.ts (1)
Security(6-51)apps/server/src/middleware/authentication.ts (2)
BEARER_AUTH(7-7)ADMIN_SCOPE(9-9)apps/server/src/services/populateTableService.ts (1)
populateTableService(4-39)
apps/server/src/services/floorService.ts (1)
apps/server/prisma/index.ts (1)
prisma(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Seer Code Review
🔇 Additional comments (69)
apps/visualizer/src/components/ui-layout/LoadingText.tsx (1)
15-15: ✓ ClassName utility ordering aligns with Biome consistency rules.The reordering of Tailwind utilities (grouping positioning classes before transforms) is a common best practice for readability and consistency. This change is part of the broader code-quality improvements mentioned in the PR objectives.
.devcontainer/devcontainer.json (2)
26-26: Good addition for Prisma development support.Adding the Prisma extension will provide developers with syntax highlighting, IntelliSense, and linting for Prisma schema files, improving the development experience.
59-61: LGTM! Consistent formatter configuration.The YAML formatter configuration follows the same pattern as other language-specific formatters in this file and will ensure consistent YAML formatting across the project with format-on-save enabled.
apps/web/src/components/floor-switcher/mobile/FloorSwitcherDisplayMobile.tsx (1)
95-95: LGTM! Non-functional formatting change.The reordering of Tailwind utility classes maintains semantic equivalence with no impact on rendering or behavior. This appears to be an automated formatting change from Biome linting rules, which is consistent with the PR's objective.
syncpack.config.ts (1)
1-1: LGTM! Well-documented Biome directive and improved comment consistency.The changes appropriately address Biome linting requirements:
- The
biome-ignoredirective on line 1 is correctly formatted and includes a clear justification with a reference URL, following best practices for suppressing lint rules.- The comment consolidation on line 16 improves consistency with other single-line comments in the file.
Both changes enhance code quality without affecting functionality.
Also applies to: 16-16
apps/server/src/controllers/poiController.ts (1)
13-16: LGTM!The import statements correctly use
.tsextensions, which aligns with ESM module resolution requirements and is consistent with the pattern applied across other controllers in this PR.apps/server/src/controllers/dropTablesController.ts (2)
9-9: Explicitpublicmodifier aligns with Biome rules.The
publicmodifier is now explicitly declared. While TypeScript class methods are public by default (making this technically redundant), explicit visibility modifiers improve code clarity and likely satisfy the Biome linting rules mentioned in the PR title.
2-3: No action needed on the imports. The.tsextensions in imports are already the established convention throughout the codebase and are explicitly supported by the TypeScript configuration (allowImportingTsExtensions: true). The project uses Bun as its runtime, which natively handles.tsimports.Likely an incorrect or invalid review comment.
apps/server/src/controllers/populateTableController.ts (2)
11-11: LGTM! Explicit visibility modifiers align with Biome linting rules.The addition of explicit
publicmodifiers on all controller methods improves code clarity and aligns with the Biome linting rules being enforced in this PR. While TypeScript class members are public by default, making visibility explicit is a good practice for consistency and maintainability.Also applies to: 17-17, 23-23, 29-35, 41-41
3-4: No changes needed to import extensions.The explicit
.tsfile extensions are correct for this project. Thetsconfig.jsonexplicitly enables them viaallowImportingTsExtensions: trueand usesmoduleResolution: "bundler"withverbatimModuleSyntax: truefor Bun runtime compatibility with ESM modules.apps/server/src/controllers/edgeController.ts (2)
23-23: LGTM: Explicit visibility modifiers improve code clarity.Adding
publicmodifiers makes the method visibility explicit, which improves code readability and documentation even though TypeScript methods are public by default. This aligns with the Biome linting rules mentioned in the PR.Also applies to: 41-41, 55-55, 105-105
12-16: No action needed—.tsextensions are correctly configured for this project.The root
tsconfig.jsonexplicitly enablesallowImportingTsExtensions: trueand usesmoduleResolution: "bundler", which are designed to support.tsextensions in import paths. Combined with Bun's TypeScript handling (module: "Preserve"andverbatimModuleSyntax: true), these imports will build and run correctly without issues.apps/web/src/components/toolbar/SearchResults.tsx (2)
20-20: LGTM! Proper null handling throughout.The addition of
| nullto thelabelPositiontype is well-handled. All usage sites (lines 156, 162, and 176-178) properly check for null or use optional chaining to safely access the properties.
45-48: API client migration is correct and response schema is compatible.The migration from
$rapito$apiuses the properly typed OpenAPI client. The/searchendpoint response schema includes all fields expected bySearchResultProps:id,nameWithSpace,fullNameWithSpace,alias,type,labelPosition, andfloor, with matching types and optional modifiers.apps/server/src/controllers/nodeController.ts (4)
25-25: Good addition of explicit public modifiers.The explicit
publicmodifiers improve code clarity and are likely part of the Biome rules enforcement mentioned in the PR title. While TypeScript class members are public by default, making this explicit is a good practice for readability.Also applies to: 47-47, 56-56
36-37: Good addition: automatic edge creation for neighbors.The automatic creation of edges to neighbors when a node is created ensures graph connectivity and maintains data consistency. The logic correctly extracts neighbor IDs from the
nodeInfo.neighborsobject and creates bidirectional edges viaedgeService.createEdges.
39-41: WebSocket broadcast logic is correct (pending socketId fix).The WebSocket broadcast implementations for all three operations (create, delete, update) are well-structured with appropriate event names and payloads. However, these will only work correctly once the socketId extraction bug (flagged above) is fixed.
Also applies to: 51-51, 65-66
13-18: Remove this concern—.tsextensions are intentionally configured.The project explicitly enables
.tsimports via"allowImportingTsExtensions": truein tsconfig.json, paired with"moduleResolution": "bundler"and Bun as the runtime. This configuration correctly supports.tsfile imports. No action needed.Likely an incorrect or invalid review comment.
apps/server/Dockerfile (1)
36-36: LGTM!The swagger-ui-dist path correction aligns with the monorepo structure where dependencies are installed in workspace package directories.
apps/server/src/controllers/buildingController.ts (2)
7-33: LGTM: Explicitpublicmodifiers align with Biome rules.The addition of explicit
publicmodifiers to all controller methods is consistent with the PR objective of enforcing Biome linting rules. While these modifiers are technically redundant in TypeScript (methods are public by default), making accessibility explicit improves code clarity and aligns with the project's coding standards.
2-2: No action required. The import with the.tsextension is properly supported by the project's TypeScript configuration ("allowImportingTsExtensions": truewith"moduleResolution": "bundler"), so there are no build compatibility concerns. This change is safe and intentional.Likely an incorrect or invalid review comment.
apps/web/src/components/floor-switcher/FloorSwitcher.tsx (1)
62-62: LGTM! Non-functional Tailwind class reordering.This reordering of Tailwind utility classes is a non-functional change that aligns with Biome linting rules introduced in this PR. The positioning and styling behavior remain unchanged.
.github/CONTRIBUTING.md (1)
40-40: LGTM! More maintainable CI documentation.The updated wording makes the documentation more flexible and future-proof by covering all "Check /" prefixed workflows without needing updates when new checks are added.
apps/visualizer/src/components/ui-layout/MainDisplay.tsx (1)
68-68: LGTM! Non-functional Tailwind class reordering.This change reorders Tailwind utilities without affecting the rendered layout or behavior, consistent with the Biome linting rules applied across the PR.
apps/visualizer/src/components/ui-layout/NavBar.tsx (1)
19-19: LGTM! Non-functional Tailwind class reordering.This reordering maintains the same visual output while conforming to the Biome linting standards introduced in this PR.
apps/server/package.json (1)
17-17: Version 3.0.0 exists and is the latest published version of tinyqueue (published Aug 12, 2024). No known security vulnerabilities have been reported for this version across npm, Snyk, and other security scanning sources.apps/visualizer/src/components/shared/ErrorDisplay.tsx (1)
7-7: Class order enforcement by Biome linting.The Tailwind utility classes have been reordered per Biome's formatting rules. This is a purely stylistic change with no impact on visual output or behavior.
apps/visualizer/src/components/map-view/PlacementPanel.tsx (1)
140-140: Biome-enforced Tailwind class reordering.The className on the root container has been reordered to follow Biome's consistent class ordering rules. No functional or visual impact.
apps/visualizer/src/components/ui-layout/FloorSwitcher.tsx (1)
112-112: Biome-enforced Tailwind class reordering.The className on the root container has been reordered per Biome's formatting rules. The visual layout and component behavior remain unchanged.
packages/common/src/types/floorTypes.ts (1)
6-13: LGTM!The
FloorInfointerface is well-structured, cleanly extendingFloorwith geographic and transformation properties needed for navigation features.packages/common/src/types/graphTypes.ts (1)
69-74: Verify:addCosttyped asstringbut represents a cost factor.The docstring describes
addCostas "Additional cost factor" which typically suggests a numeric value. Isstringintentional here (e.g., representing a category like "elevator" or "stairs"), or should this benumber?apps/web/src/hooks/useMapRegionChange.ts (1)
72-74: LGTM!The field reference is correctly updated from
node_idtonodeId, consistent with the type definition changes ingraphTypes.ts.apps/server/src/services/floorService.ts (2)
258-270: LGTM!Good addition of floor existence validation with a descriptive error message including the floor code.
355-360: LGTM!The
idfield addition aligns with the updatedGeoNodeinterface ingraphTypes.ts.apps/web/src/components/nav/NavCard.tsx (1)
93-105: LGTM!The Tailwind class reordering is a non-functional style consistency improvement.
package.json (1)
17-17: remeda is correctly added and actively used. The library is imported inscripts/biome/generate-biome-rules.tswheremergeDeepis used to merge biome configuration objects. The devDependency classification is appropriate since this utility script is part of the build tooling.apps/web/src/hooks/useLocationParams.ts (1)
5-5: LGTM! Clean refactoring to use utility function.The extraction of floor code construction into
buildFloorCodeimproves code reuse and maintainability. The utility correctly handles edge cases (undefined values, "outside" building code) and returnsnullwhen appropriate, which integrates well with the query'senabledguard.Also applies to: 33-33
apps/web/src/types/navTypes.ts (1)
19-19: LGTM! Improved naming consistency.The rename from
node_idtonodeIdfollows JavaScript/TypeScript camelCase conventions and aligns with theInstructiontype definition inpackages/common/src/types/graphTypes.ts. The AI summary indicates all dependent UI components were updated consistently..husky/pre-commit (1)
5-6: LGTM! New pre-commit checks added.The additions of
syncpackandmarkdownlint-cli2checks align with the PR's goal of enhancing development tooling. Both commands will correctly abort the commit on failure due toset -e.However,
markdownlint-cli2is already present inpackage.jsonas a dev dependency (^0.20.0), so the package is cached locally. Usingbunxon cached packages executes quickly, so the suggested optimization to add it as a dev dependency is already in place.apps/web/src/components/nav/NavDirectionsList.tsx (1)
97-97: LGTM! Field name standardization to camelCase.The rename from
node_idtonodeIdaligns with JavaScript/TypeScript naming conventions and is consistently applied across both occurrences in this component.Also applies to: 197-197
packages/common/src/types/index.ts (1)
11-11: LGTM! Waypoint export added consistently.The export follows the established pattern in this index file.
packages/common/src/types/waypoint.ts (1)
1-15: LGTM! Well-structured discriminated union.The
WayPointtype is correctly defined as a discriminated union with atypediscriminator, providing excellent type safety for pathfinding operations. The structure aligns well with the use cases described in the related utilities.biome.jsonc (4)
1-32: LGTM! Well-structured Biome configuration.The VCS integration, file inclusion/exclusion patterns, formatter, and assist configurations are appropriately set up. The exclusion of generated files (
routeTree.gen.ts,openapi.d.ts) and build artifacts is correct.
330-335: LGTM! Explicit accessibility modifiers enforced.The
useConsistentMemberAccessibilityrule configuration correctly enforces explicit accessibility modifiers (public,private,protected) on class members, improving code clarity.
352-357: LGTM! Separated type imports enforced.The
useImportTypeconfiguration with"separatedType"style correctly enforcesimport typestatements for type-only imports, which improves build performance and makes the code's intent clearer.
477-486: LGTM! Parser configurations appropriate for the tech stack.The JavaScript parser's
unsafeParameterDecoratorsEnabledand CSS parser'stailwindDirectivesare correctly configured to support TypeScript decorators and Tailwind CSS syntax respectively..github/workflows/check.yml (3)
22-27: LGTM! Markdown linting job added.The markdownlint job using
markdownlint-cli2-actionis correctly configured and follows best practices.
29-42: Verify partial dependency installation for syncpack job.The syncpack job only installs the
syncpackpackage rather than all project dependencies. If syncpack has peer dependencies or requires workspace configuration, this may fail.Consider whether you need:
- name: Install dependencies - run: bun install syncpack + run: bun installOr verify that installing only
syncpackis sufficient for your use case.
44-57: Verify partial dependency installation for biome-rules-check job.Similar to the syncpack job, this only installs
remedarather than all dependencies. If the check script (scripts/biome/check-biome-config.ts) requires other packages or workspace dependencies, this may fail.Consider whether you need:
- name: Install dependencies - run: bun install remeda + run: bun installOr verify that installing only
remedais sufficient.apps/web/src/hooks/useNavigationParams.ts (2)
7-7: LGTM! Good use of helper function.Centralizing floor code construction through
buildFloorCodeimproves maintainability and ensures consistent handling of edge cases (undefined values, "outside" building).Also applies to: 40-40
125-139: API client migration is complete and consistent.All uses of
$rapihave been successfully migrated to$api. The reviewed change in useNavigationParams.ts correctly imports from@/api/clientand is consistent with the rest of the codebase. The legacyrustClient.tsfile is unused and can be considered for removal.apps/web/src/components/nav/NavOverlay.tsx (3)
32-32: LGTM! Field rename consistent with type updates.The update from
node_idtonodeIdaligns with the instruction type changes across the codebase.
40-52: Good defensive programming for Enter instructions.The
!node.floorguard prevents potential null reference errors when checkingbuildingCode. Treating nodes without floor data as outside is a reasonable fallback.
53-85: LGTM! Consistent instruction handling.Exit instruction includes the same defensive guards, and Forward/Arrive instructions properly handle the path array with optional chaining. Field renames are consistent.
apps/server/src/utils/search/parse.ts (2)
6-15: LGTM! Standard trigram implementation.The trigram generation logic correctly handles padding and sliding window extraction.
22-35: The space preservation in the regex is intentional for phrase-aware matching. TheparseQueryfunction generates trigrams that can match across word boundaries, enabling queries like "new york" to match documents containing that multi-word phrase. This is evidenced by the explicit use ofnameWithSpaceandfullNameWithSpacefields inbuild.ts(lines 56-57 for buildings, lines 56-57 for rooms), which are specifically passed toparseQueryto index cross-word trigrams. No changes needed.Likely an incorrect or invalid review comment.
apps/web/src/components/nav/NavLine.tsx (1)
91-91: LGTM! Consistent field rename throughout.All references to
node_idhave been correctly updated tonodeId, including usage in find predicates, variable assignments, and useEffect dependency arrays. The changes maintain functional equivalence.Also applies to: 110-110, 127-127, 149-149, 277-277
apps/web/src/utils/floorUtils.ts (2)
39-41: LGTM!Good addition of the
buildingCode === "outside"guard. This prevents generating invalid floor codes like"outside-1"and aligns with the broader refactoring to handle outside contexts safely.
43-46: LGTM!The signature update to accept
Floor | undefinedwith astring | nullreturn type properly reflects the nullability semantics. The implementation correctly delegates tobuildFloorCodefor consistent floor code construction.apps/web/src/components/info-cards/nav-card-desktop/NavCardDesktop.tsx (2)
198-202: LGTM!The field rename from
node_idtonodeIdaligns with the updatedInstructiontype definition in@cmumaps/common.
417-419: LGTM!Proper use of optional chaining for accessing
floor?.buildingCodeto guard against undefined floor values.apps/server/src/services/pathService.ts (1)
43-66: LGTM!The graph construction logic correctly maps database nodes to the
GeoNodestructure with proper handling of floor information for both regular floors and "outside" nodes.packages/common/src/utils/coordinates.ts (3)
12-13: LGTM!Exporting these constants enables reuse across the codebase while maintaining the Pittsburgh-specific coordinate conversion ratios.
124-134: LGTM!The planar distance approximation using the latitude/longitude ratios is appropriate for the Pittsburgh region's scale.
145-164: LGTM!The angle calculation using the cross product (for sign) and dot product correctly determines the turning direction. Negative values indicate left turns and positive values indicate right turns, as documented.
apps/server/src/utils/search/rank.ts (2)
13-30: LGTM!Standard BM25 implementation with reasonable parameter choices (k=1.2, b=0.2). The formula correctly computes the IDF and TF components.
36-47: LGTM!The distance-weighted scoring with logarithmic decay provides a sensible proximity boost. The
+ 10.0prevents division issues for very close locations, and+ 1.0ensures the denominator is always > 1.apps/server/src/services/searchService.ts (1)
9-43: Module-level cache has no invalidation mechanism.The
searchContextCacheis never invalidated. If buildings or rooms are added/updated/deleted, the search service will continue serving stale results until server restart.Consider whether cache invalidation is needed for this use case. If data changes infrequently, this may be acceptable, but you may want to add a manual invalidation function or TTL-based refresh for production use.
apps/server/src/controllers/floorController.ts (1)
10-51: LGTM!The addition of explicit
publicmodifiers on controller methods improves code clarity and aligns with theuseConsistentMemberAccessibilityBiome rule configured in this PR.
| # Copy only what's needed to run the server | ||
| COPY --from=builder --chown=server:nodejs /app/apps/server/dist ./apps/server/dist | ||
| COPY --from=builder --chown=server:nodejs /app/node_modules/prisma /tmp/prisma-engines | ||
| COPY --from=builder --chown=server:nodejs /app/apps/server/node_modules/prisma /tmp/prisma-engines |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for Prisma environment variable configuration in config files
rg -i "PRISMA.*ENGINE|PRISMA_BINARIES" -t json -t yaml -t env
# Check for .env files
fd -e env -x cat {}
# Check Prisma schema
fd -e prisma -x cat {}
# Check if /tmp/prisma-engines path is referenced anywhere
rg "/tmp/prisma-engines" -A 2 -B 2
# Check for Prisma Client instantiation with custom configuration
rg "new PrismaClient" -A 3 -B 1
# Get full Dockerfile context around line 40
cat -n apps/server/Dockerfile | head -50Repository: ScottyLabs/cmumaps
Length of output: 4519
🏁 Script executed:
# Check for any startup/entry scripts in server
find apps/server -type f \( -name "*.sh" -o -name "*.js" -o -name "*.ts" \) | head -20
# Check server.js/server.ts for environment setup
fd server -e ts -e js apps/server/dist -x head -20 {}
# Check if any environment variables are set in the build process
rg "PRISMA" --no-heading
# Check for any docker-compose or environment setup
fd -e "docker-compose*" -e "*.env*" -x cat {}
# Check entire Dockerfile for ENV declarations
cat apps/server/Dockerfile | grep -i "ENV\|PRISMA"Repository: ScottyLabs/cmumaps
Length of output: 1199
🏁 Script executed:
# Check env.ts for Prisma-related environment variables
cat apps/server/src/env.ts
# Check server.ts startup code
cat apps/server/src/server.ts
# Check if there's any configuration in package.json scripts
grep -A 5 -B 5 "db-generate\|start\|dev" apps/server/package.json || echo "checking root"
cat package.json | grep -A 10 -B 2 "scripts"Repository: ScottyLabs/cmumaps
Length of output: 5249
Fix Prisma engine path configuration in Dockerfile.
Copying Prisma to /tmp/prisma-engines without configuring PrismaClient to use this path will cause runtime failures. Either:
-
Set the
PRISMA_QUERY_ENGINE_BINARYenvironment variable in the Dockerfile:ENV PRISMA_QUERY_ENGINE_BINARY=/tmp/prisma-engines/libquery_engine... -
Or copy the entire
node_modulesdirectory to the runtime image so Prisma finds engines in their default location.
Currently, there are no environment variables or Prisma configuration that tells PrismaClient to look in /tmp/prisma-engines, and the Prisma Client instantiation uses default settings. This will cause "Query engine binary not found" errors when the server starts.
🤖 Prompt for AI Agents
In @apps/server/Dockerfile at line 40, The Dockerfile currently copies Prisma
engines to /tmp/prisma-engines (COPY --from=builder --chown=server:nodejs
/app/apps/server/node_modules/prisma /tmp/prisma-engines) but PrismaClient is
not configured to look there; either set the PRISMA_QUERY_ENGINE_BINARY env var
to point at the engine binary under /tmp/prisma-engines (add an ENV
PRISMA_QUERY_ENGINE_BINARY=... line) so PrismaClient finds the engine, or
instead copy the full node_modules for Prisma into the runtime image so engines
remain in their default location; update the Dockerfile to perform one of these
fixes and ensure PrismaClient will locate the binary at runtime.
| const { floorCode, nodeInfo } = body; | ||
| const nodeId = req.params.nodeId; | ||
| const socketId = req.socketId; | ||
| const { nodeId, socketId } = req.params; |
There was a problem hiding this comment.
Critical: socketId extracted from wrong source.
The code destructures socketId from req.params, but the requireSocketId middleware sets it as req.socketId (not in params). The route definitions (/:nodeId) only include nodeId as a path parameter. This will cause socketId to be undefined at runtime, breaking the WebSocket broadcasts.
🔎 Proposed fix for all three methods
Fix for createNode (line 30):
- const { nodeId, socketId } = req.params;
+ const { nodeId } = req.params;
+ const socketId = req.socketId;Fix for deleteNode (line 48):
- const { nodeId, socketId } = req.params;
+ const { nodeId } = req.params;
+ const socketId = req.socketId;Fix for updateNode (line 61):
- const { nodeId, socketId } = req.params;
+ const { nodeId } = req.params;
+ const socketId = req.socketId;Also applies to: 48-48, 61-61
🤖 Prompt for AI Agents
In @apps/server/src/controllers/nodeController.ts at line 30, The handlers
createNode, deleteNode, and updateNode in nodeController.ts are currently
destructuring socketId from req.params but the requireSocketId middleware
attaches it as req.socketId; change each handler to stop reading socketId from
req.params and instead read socketId from req.socketId (and update any typings
or interfaces for the request object if necessary) so broadcasts use the correct
socketId value.
| @Body() body: { poiInfo: PoiInfo }, | ||
| ) { | ||
| const poiId = req.params.poiId; | ||
| const { poiId, socketId } = req.params; |
There was a problem hiding this comment.
Critical: socketId is incorrectly extracted from req.params.
The requireSocketId middleware sets socketId on req.socketId, not on req.params. Since the route is /:poiId (without a :socketId param), socketId will be undefined, causing WebSocket broadcasts to fail silently.
🔎 Proposed fix
- const { poiId, socketId } = req.params;
+ const { poiId } = req.params;
+ const socketId = req.socketId;📝 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.
| const { poiId, socketId } = req.params; | |
| const { poiId } = req.params; | |
| const socketId = req.socketId; |
🤖 Prompt for AI Agents
In @apps/server/src/controllers/poiController.ts at line 27, The handler
incorrectly reads socketId from req.params; change the extraction so poiId still
comes from req.params (e.g., keep const { poiId } = req.params) and obtain
socketId from req.socketId (set by requireSocketId middleware) before using it
for broadcasts; update any references that assume socketId came from params to
use the new socketId variable so WebSocket broadcasts receive the correct id.
| const socketId = req.socketId; | ||
|
|
||
| public async deletePoi(@Request() req: ExpressRequest) { | ||
| const { poiId, socketId } = req.params; |
There was a problem hiding this comment.
Same socketId extraction issue.
Apply the same fix here: extract socketId from req.socketId instead of req.params.
🔎 Proposed fix
- const { poiId, socketId } = req.params;
+ const { poiId } = req.params;
+ const socketId = req.socketId;📝 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.
| const { poiId, socketId } = req.params; | |
| const { poiId } = req.params; | |
| const socketId = req.socketId; |
🤖 Prompt for AI Agents
In @apps/server/src/controllers/poiController.ts at line 38, In the POI
controller handler replace destructuring that reads socketId from req.params
(const { poiId, socketId } = req.params) so that poiId still comes from
req.params but socketId is read from req.socketId; update the code paths in the
same function (the handler in poiController.ts) to use the new socketId source
and remove reliance on params.socketId.
| @Body() body: { poiType: PoiType }, | ||
| ) { | ||
| const poiId = req.params.poiId; | ||
| const { poiId, socketId } = req.params; |
There was a problem hiding this comment.
Same socketId extraction issue.
Apply the same fix here as well.
🔎 Proposed fix
- const { poiId, socketId } = req.params;
+ const { poiId } = req.params;
+ const socketId = req.socketId;📝 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.
| const { poiId, socketId } = req.params; | |
| const { poiId } = req.params; | |
| const socketId = req.socketId; |
🤖 Prompt for AI Agents
In @apps/server/src/controllers/poiController.ts at line 50, The destructuring
currently pulls socketId from req.params which is incorrect; in poiController.ts
keep poiId from req.params (const { poiId } = req.params) and obtain socketId
from the query (const { socketId } = req.query) or wherever the rest of the app
supplies it (e.g., req.body) so the socketId variable is read from the correct
source before use in the controller.
| export const geoNodeToNavPathNode = (geoNode: GeoNode): NavPathNode => { | ||
| return { | ||
| neighbors: geoNode.neighbors, | ||
| coordinate: geoNode.pos, | ||
| roomId: geoNode.roomId || "outside", | ||
| id: geoNode.id, | ||
| floor: geoNode.floor | ||
| ? { | ||
| buildingCode: geoNode.floor.buildingCode, | ||
| level: geoNode.floor.level, | ||
| } | ||
| : undefined, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how roomId is used downstream
rg -n "roomId" --type ts -C 3 apps/web/src apps/server/src 2>/dev/null | head -150Repository: ScottyLabs/cmumaps
Length of output: 9422
🏁 Script executed:
#!/bin/bash
# Find NavPathNode type definition
rg -n "type NavPathNode|interface NavPathNode" --type tsRepository: ScottyLabs/cmumaps
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Check how "outside" is used in the codebase
rg -n '"outside"' --type ts | head -50Repository: ScottyLabs/cmumaps
Length of output: 2426
🏁 Script executed:
#!/bin/bash
# Get the full NavPathNode interface definition
rg -n "interface NavPathNode" -A 10 --type ts packages/common/src/types/graphTypes.tsRepository: ScottyLabs/cmumaps
Length of output: 266
🏁 Script executed:
#!/bin/bash
# Check if floor is optional in NavPathNode
rg -n "floor" --type ts apps/web/src/types/navTypes.tsRepository: ScottyLabs/cmumaps
Length of output: 80
🏁 Script executed:
#!/bin/bash
# Look for any code that checks if roomId is "outside"
rg -n 'roomId.*===.*"outside"|"outside".*===.*roomId' --type tsRepository: ScottyLabs/cmumaps
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check geoNode definition to understand when roomId could be null
rg -n "interface GeoNode|type GeoNode" -A 15 --type tsRepository: ScottyLabs/cmumaps
Length of output: 2516
🏁 Script executed:
#!/bin/bash
# Check how NavPathNode is actually used/consumed downstream
rg -n "NavPathNode" --type ts -C 2 | grep -A 3 -B 3 "roomId" | head -40Repository: ScottyLabs/cmumaps
Length of output: 44
Use null or make roomId optional instead of using "outside" as fallback.
The fallback "outside" is semantically incorrect. Per the GeoNode definition, roomId: null means the node is "not associated with any room," which is a valid state. However, "outside" represents an outdoor building/floor area, not the absence of a room. Consider either making NavPathNode.roomId optional (string | undefined) or using null as the actual fallback value to preserve the semantic meaning of unassociated nodes.
🤖 Prompt for AI Agents
In @packages/common/src/utils/coordinates.ts around lines 166 - 179, The
function geoNodeToNavPathNode currently maps geoNode.roomId to the literal
"outside"; change this so NavPathNode preserves the semantic absence of a room
by either setting roomId to geoNode.roomId ?? null or making NavPathNode.roomId
optional and assigning roomId: geoNode.roomId, and update the NavPathNode type
definition to accept null (string | null) or undefined accordingly; modify any
callers that assume the "outside" string to handle null/undefined instead.
| // Check if the Biome rules are up to date | ||
|
|
||
| import fs from "node:fs/promises"; | ||
| import process from "node:process"; | ||
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | ||
|
|
||
| const allRules = await getAllRules(); | ||
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | ||
| if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) { | ||
| console.debug("Biome config is up to date"); | ||
| process.exit(0); | ||
| } else { | ||
| console.error("Biome config is not up to date"); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for async operations.
The script lacks error handling for network requests (via getAllRules()), file I/O, and JSON parsing. Any failure will result in an unhandled promise rejection.
🔎 Suggested error handling
+try {
const allRules = await getAllRules();
const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8"));
if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) {
console.debug("Biome config is up to date");
process.exit(0);
} else {
console.error("Biome config is not up to date");
process.exit(1);
}
+} catch (error) {
+ console.error("Failed to check Biome config:", error);
+ process.exit(1);
+}📝 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.
| // Check if the Biome rules are up to date | |
| import fs from "node:fs/promises"; | |
| import process from "node:process"; | |
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | |
| const allRules = await getAllRules(); | |
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | |
| if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) { | |
| console.debug("Biome config is up to date"); | |
| process.exit(0); | |
| } else { | |
| console.error("Biome config is not up to date"); | |
| process.exit(1); | |
| } | |
| // Check if the Biome rules are up to date | |
| import fs from "node:fs/promises"; | |
| import process from "node:process"; | |
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | |
| try { | |
| const allRules = await getAllRules(); | |
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | |
| if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) { | |
| console.debug("Biome config is up to date"); | |
| process.exit(0); | |
| } else { | |
| console.error("Biome config is not up to date"); | |
| process.exit(1); | |
| } | |
| } catch (error) { | |
| console.error("Failed to check Biome config:", error); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In @scripts/biome/check-biome-config.ts around lines 1 - 15, Wrap the async
operations in check-biome-config.ts (the await getAllRules(), await
fs.readFile(BIOME_CONFIG_PATH, ...) and JSON.parse(...)) in a try/catch so
failures are caught; on error call processLogger or console.error with the
caught error (include message/stack) and process.exit(1). Specifically, surround
the logic that uses getAllRules, fs.readFile and JSON.parse, validate
biomeConfig.linter and rules exist before comparing, and ensure any thrown error
is logged and leads to a non-zero exit instead of an unhandled rejection.
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | ||
|
|
||
| const allRules = await getAllRules(); | ||
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); |
There was a problem hiding this comment.
JSON.parse cannot handle JSONC files with comments.
Same issue as in write-biome-config.ts: JSON.parse() will fail if biome.jsonc contains comments. Additionally, accessing biomeConfig.linter.rules without validation can throw if the structure is missing.
🔎 Proposed fix
+import { parse as parseJsonc } from "jsonc-parser";
+
const allRules = await getAllRules();
-const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8"));
+const biomeConfigText = await fs.readFile(BIOME_CONFIG_PATH, "utf-8");
+const biomeConfig = parseJsonc(biomeConfigText);
+
+if (!biomeConfig?.linter?.rules) {
+ console.error("Invalid Biome config structure: missing linter.rules");
+ process.exit(1);
+}
+
if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) {📝 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.
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | |
| import { parse as parseJsonc } from "jsonc-parser"; | |
| const allRules = await getAllRules(); | |
| const biomeConfigText = await fs.readFile(BIOME_CONFIG_PATH, "utf-8"); | |
| const biomeConfig = parseJsonc(biomeConfigText); | |
| if (!biomeConfig?.linter?.rules) { | |
| console.error("Invalid Biome config structure: missing linter.rules"); | |
| process.exit(1); | |
| } | |
| if (JSON.stringify(biomeConfig.linter.rules) === JSON.stringify(allRules)) { |
🤖 Prompt for AI Agents
In @scripts/biome/check-biome-config.ts at line 8, Replace the raw JSON.parse
call on the BIOME_CONFIG_PATH file with a JSONC-aware parse (e.g., use a jsonc
parser or strip comments before parsing) so files with comments (biome.jsonc)
don't crash; after parsing into biomeConfig, defensively check that
biomeConfig.linter and biomeConfig.linter.rules exist and are the expected types
before accessing them (refactor any direct accesses to biomeConfig.linter.rules
into guarded checks and throw or log a clear error if the structure is missing).
| import fs from "node:fs/promises"; | ||
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | ||
|
|
||
| const allRules = await getAllRules(); | ||
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | ||
| biomeConfig.linter.rules = allRules; | ||
| await fs.writeFile( | ||
| BIOME_CONFIG_PATH, | ||
| `${JSON.stringify(biomeConfig, null, 2)}\n`, | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for network and file system operations.
The script performs several operations that can fail (network fetch via getAllRules(), file I/O, JSON parsing) but has no error handling. If any operation fails, the script will crash with an unhelpful error message.
🔎 Suggested error handling
+try {
const allRules = await getAllRules();
const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8"));
biomeConfig.linter.rules = allRules;
await fs.writeFile(
BIOME_CONFIG_PATH,
`${JSON.stringify(biomeConfig, null, 2)}\n`,
);
+ console.log("✓ Biome config updated successfully");
+} catch (error) {
+ console.error("Failed to update Biome config:", error);
+ process.exit(1);
+}📝 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.
| import fs from "node:fs/promises"; | |
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | |
| const allRules = await getAllRules(); | |
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | |
| biomeConfig.linter.rules = allRules; | |
| await fs.writeFile( | |
| BIOME_CONFIG_PATH, | |
| `${JSON.stringify(biomeConfig, null, 2)}\n`, | |
| ); | |
| import fs from "node:fs/promises"; | |
| import { BIOME_CONFIG_PATH, getAllRules } from "./generate-biome-rules.ts"; | |
| try { | |
| const allRules = await getAllRules(); | |
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | |
| biomeConfig.linter.rules = allRules; | |
| await fs.writeFile( | |
| BIOME_CONFIG_PATH, | |
| `${JSON.stringify(biomeConfig, null, 2)}\n`, | |
| ); | |
| console.log("✓ Biome config updated successfully"); | |
| } catch (error) { | |
| console.error("Failed to update Biome config:", error); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In @scripts/biome/write-biome-config.ts around lines 1 - 10, The script lacks
error handling around async/network and file operations; wrap the main sequence
(await getAllRules(), fs.readFile(BIOME_CONFIG_PATH), JSON.parse, and
fs.writeFile) in a try/catch, validate that the parsed object has a
biomeConfig.linter object before assigning biomeConfig.linter.rules = allRules
(create it if missing), and on error log a clear message including the caught
error via console.error or processLogger.error and terminate with
process.exit(1); also consider catching specific JSON.parse failures to report
malformed config and rethrow or exit appropriately.
| const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8")); | ||
| biomeConfig.linter.rules = allRules; |
There was a problem hiding this comment.
JSON.parse cannot handle JSONC files with comments.
The file extension .jsonc indicates JSON with Comments format, but JSON.parse() will throw a syntax error if the config file contains any comments. Additionally, there's no validation that biomeConfig.linter exists before accessing .rules.
🔎 Proposed fix
Consider using a JSONC-compatible parser or strip comments before parsing:
+import { parse as parseJsonc } from "jsonc-parser";
+
const allRules = await getAllRules();
-const biomeConfig = JSON.parse(await fs.readFile(BIOME_CONFIG_PATH, "utf-8"));
+const biomeConfigText = await fs.readFile(BIOME_CONFIG_PATH, "utf-8");
+const biomeConfig = parseJsonc(biomeConfigText);
+if (!biomeConfig.linter) {
+ biomeConfig.linter = {};
+}
biomeConfig.linter.rules = allRules;Alternatively, if the file should never contain comments, rename it to .json.
🤖 Prompt for AI Agents
In @scripts/biome/write-biome-config.ts around lines 5 - 6, The code uses
JSON.parse on the file read at BIOME_CONFIG_PATH which may be JSONC (comments)
and also assumes biomeConfig.linter exists; replace the parse with a JSONC-aware
parse (or strip comments before parsing) when reading via fs.readFile, and
ensure you validate/create biomeConfig.linter before assigning
biomeConfig.linter.rules = allRules (e.g., check biomeConfig && typeof
biomeConfig === "object" and if (!biomeConfig.linter) set biomeConfig.linter =
{}), keeping references to BIOME_CONFIG_PATH, biomeConfig, allRules, and the
fs.readFile usage so the change is made where the file is read and modified.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.