lint: enable detekt rule: MagicNumber#335
Open
AdrianLeeElder wants to merge 15 commits intodevelopfrom
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnabled Detekt's style.MagicNumber in backend and ee/backend configs and replaced many hardcoded numeric literals across the codebase with named constants (file-level or companion objects); no functional control-flow or public API signatures were changed. Changes
Sequence Diagram(s)(omitted — changes are configuration and centralized constant extraction without a new multi-component control flow that benefits from a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
backend/src/main/kotlin/com/moneat/security/routes/SecurityRoutes.kt (1)
108-119:⚠️ Potential issue | 🟠 MajorUse
suspendRunCatchingin this suspend function (Line 109).
runCatchinginside a suspend function may swallowCancellationException; usesuspendRunCatchingto preserve cancellation behavior.Suggested fix
+import com.moneat.utils.suspendRunCatching @@ private suspend fun executeChQuery(query: String): List<JsonObject>? { - return runCatching { + return suspendRunCatching { val response = ClickHouseClient.execute(query) if (response.status.value !in HTTP_SUCCESS_MIN..HTTP_SUCCESS_MAX) { logger.warn { "ClickHouse query failed: ${response.status}" } return null } parseJsonEachRow(response.bodyAsText()) }.getOrElse { logger.warn { "ClickHouse query error: ${it.message}" } null } }Per coding guidelines: "Use
suspendRunCatchingfromcom.moneat.utilsinstead ofrunCatchingin suspend/coroutine functions to preserveCancellationExceptionbehavior."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/security/routes/SecurityRoutes.kt` around lines 108 - 119, The suspend function executeChQuery uses runCatching which can swallow CancellationException; replace runCatching with suspendRunCatching (from com.moneat.utils) in executeChQuery so coroutine cancellation is preserved, keeping the existing success path (check response.status, log warn and return null on non-2xx, call parseJsonEachRow) and the existing getOrElse error logging/return null behavior unchanged.backend/src/main/kotlin/com/moneat/auth/services/RefreshTokenService.kt (1)
37-41:⚠️ Potential issue | 🟠 MajorTwo
3600literals still keepMagicNumberalive here.Line 40 and Line 102 still hardcode the hour-in-seconds value, so this file is only partially migrated right now.
♻️ Proposed fix
+private const val SECONDS_PER_HOUR = 3_600L + data class RefreshTokenResponse( val accessToken: String, val refreshToken: String, - val expiresIn: Long = 3600 // 1 hour in seconds + val expiresIn: Long = SECONDS_PER_HOUR ) @@ return RefreshTokenResponse( accessToken = accessToken, refreshToken = refreshToken, - expiresIn = ACCESS_TOKEN_EXPIRY_HOURS * 3600 + expiresIn = ACCESS_TOKEN_EXPIRY_HOURS * SECONDS_PER_HOUR )As per coding guidelines, "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))"Also applies to: 51-56, 99-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/auth/services/RefreshTokenService.kt` around lines 37 - 41, The file still uses the literal 3600 in multiple places (e.g., in the RefreshTokenResponse data class and where token expiry is computed), so replace all occurrences with a single named constant (e.g., const val ACCESS_TOKEN_EXPIRY_SECONDS = 3600) declared at the top of the file or companion object; update RefreshTokenResponse's default expiresIn to use that constant and update any methods that compute or compare expiry (look for functions that create or validate access/refresh tokens) to reference the new constant instead of the magic number.backend/src/main/kotlin/com/moneat/org/routes/IntegrationRoutes.kt (1)
59-59:⚠️ Potential issue | 🟡 MinorWildcard import violates coding guidelines.
The import
java.util.*should be replaced with explicit imports for the classes actually used (likelyBase64andUUID).🛠️ Suggested fix
-import java.util.* +import java.util.Base64 +import java.util.UUIDAs per coding guidelines: "Never use wildcard imports; always use explicit imports."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/org/routes/IntegrationRoutes.kt` at line 59, Replace the wildcard import in IntegrationRoutes.kt: remove "import java.util.*" and add explicit imports for the types actually used (e.g., import java.util.Base64 and import java.util.UUID), ensuring only referenced classes (like Base64 and UUID) are imported and any unused imports are removed to satisfy the coding guideline against wildcard imports.backend/src/main/kotlin/com/moneat/monitor/services/MonitorService.kt (1)
595-612:⚠️ Potential issue | 🟠 MajorDon’t fallback
mem_limitto1.If
mem_limitis missing or unparsable, this fabricates a 1-byte limit andmem_percentbecomes wildly incorrect instead of degrading to0.💡 Suggested fix
- val memLimit = arr[CONTAINER_COL_MEM_LIMIT].toString().replace("\"", "").toLongOrNull() ?: 1 + val memLimit = + arr.getOrNull(CONTAINER_COL_MEM_LIMIT)?.toString()?.replace("\"", "")?.toLongOrNull() ?: 0L🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/monitor/services/MonitorService.kt` around lines 595 - 612, The code currently falls back memLimit to 1 which fabricates a 1-byte limit and skews mem_percent; change the fallback for mem_limit (the variable memLimit created from arr[CONTAINER_COL_MEM_LIMIT]) to 0 instead of 1 and ensure the mem_percent calculation in the ContainerStats construction still guards with (memLimit > 0) to avoid division by zero, so when mem_limit is missing/unparsable it becomes 0 and mem_percent degrades to 0f.backend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.kt (1)
957-957:⚠️ Potential issue | 🟡 MinorInconsistent:
10_000Lshould useMICROS_PER_CENTconstant.This line still uses the hardcoded literal
10_000Lwhich is the same value as the newly introducedMICROS_PER_CENTconstant. For consistency and to satisfy the MagicNumber detekt rule, this should also use the constant.Proposed fix
- paygUsedCentsEstimate = (state.paygUsedMicros / 10_000L).toInt(), + paygUsedCentsEstimate = (state.paygUsedMicros / MICROS_PER_CENT).toInt(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.kt` at line 957, Replace the hardcoded literal 10_000L used to compute paygUsedCentsEstimate with the MICROS_PER_CENT constant for consistency with the new constant; update the expression (currently using state.paygUsedMicros / 10_000L).toInt() to use MICROS_PER_CENT (i.e., divide by MICROS_PER_CENT) in the BillingQuotaService (paygUsedCentsEstimate) so the MagicNumber rule is satisfied and the conversion is centralized.backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt (1)
675-677:⚠️ Potential issue | 🟠 MajorUse
suspendRunCatchingin this suspend function.Line 675 uses
runCatchingingetTestSummary, which is a suspend function. This interferes with proper coroutine cancellation behavior. Replace withsuspendRunCatching(already imported at line 49).🔧 Proposed fix
- val response = runCatching { + val response = suspendRunCatching { ClickHouseClient.execute(query) }.getOrNull() ?: return null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.kt` around lines 675 - 677, The getTestSummary suspend function currently wraps ClickHouseClient.execute in runCatching which can block proper coroutine cancellation; replace runCatching with suspendRunCatching (already imported) so the call becomes suspendRunCatching { ClickHouseClient.execute(query) } and keep the existing .getOrNull() ?: return null behavior to preserve the null-return on failure.
🧹 Nitpick comments (25)
backend/src/main/kotlin/com/moneat/plugins/BackgroundJobs.kt (1)
153-159: Use a telemetry-specific constant name for the interval fallback.
DEFAULT_WORKER_THREADSis semantically unrelated topulse.intervalHours. Please rename/split this to something likeDEFAULT_TELEMETRY_INTERVAL_HOURSto avoid cross-domain confusion.♻️ Suggested refactor
-private const val DEFAULT_WORKER_THREADS = 4 +private const val DEFAULT_TELEMETRY_INTERVAL_HOURS = 4 ... - ?.takeIf { it > 0 } ?: DEFAULT_WORKER_THREADS + ?.takeIf { it > 0 } ?: DEFAULT_TELEMETRY_INTERVAL_HOURS🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/plugins/BackgroundJobs.kt` around lines 153 - 159, The fallback for pulse.intervalHours is using an unrelated constant DEFAULT_WORKER_THREADS; introduce a telemetry-specific constant (e.g., DEFAULT_TELEMETRY_INTERVAL_HOURS) and use it as the fallback when computing telemetryIntervalHours in BackgroundJobs (replace DEFAULT_WORKER_THREADS in the telemetryIntervalHours assignment). Update any constant declaration/usage so PulseService(interval = telemetryIntervalHours.hours) reads from the new DEFAULT_TELEMETRY_INTERVAL_HOURS and leave other worker-thread logic untouched.backend/src/main/kotlin/com/moneat/shared/services/ArtifactCleanupService.kt (1)
58-58: Consider a single source of truth for invitation retention days.
INVITATION_EXPIRY_DAYShere and the default inOrgInvitationService.purgeOldInvitations(olderThanDays: Int = 90)can drift over time. Prefer centralizing this value (shared constant/config) or callpurgeOldInvitations()without an argument if default is authoritative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/shared/services/ArtifactCleanupService.kt` at line 58, The code in ArtifactCleanupService uses a local INVITATION_EXPIRY_DAYS while OrgInvitationService.purgeOldInvitations has a default of 90, risking drift; either remove the hardcoded INVITATION_EXPIRY_DAYS and call orgInvitationService.purgeOldInvitations() with no argument to rely on the service default, or move the expiry value into a single shared constant/config (e.g., EXPIRED_INVITATION_DAYS) consumed by both ArtifactCleanupService and OrgInvitationService.purgeOldInvitations(olderThanDays) so both reference the same source of truth.backend/src/main/kotlin/com/moneat/notifications/services/DiscordService.kt (1)
255-259: Simplify redundant severity mapping branch.Line 257 and Line 258 both return yellow, so the
"MEDIUM"case is redundant and can be collapsed intoelsefor cleaner logic.♻️ Proposed simplification
- val color = when (p.severity) { - "CRITICAL", "HIGH" -> DISCORD_COLOR_RED - "MEDIUM" -> DISCORD_COLOR_YELLOW - else -> DISCORD_COLOR_YELLOW - } + val color = when (p.severity) { + "CRITICAL", "HIGH" -> DISCORD_COLOR_RED + else -> DISCORD_COLOR_YELLOW + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/notifications/services/DiscordService.kt` around lines 255 - 259, The severity-to-color mapping in DiscordService (variable color assigned from p.severity) has a redundant "MEDIUM" branch that returns DISCORD_COLOR_YELLOW same as the else branch; simplify by removing the explicit "MEDIUM" case and let the else branch cover it so the when expression only checks "CRITICAL","HIGH" -> DISCORD_COLOR_RED and else -> DISCORD_COLOR_YELLOW, keeping the assignment to color unchanged.backend/src/main/kotlin/com/moneat/datadog/security/SecurityQueryRoutes.kt (1)
245-249: Consider extracting shared ClickHouse error handling to one helper.
executeCountandexecuteRowsnow have near-identical error logging/throw logic. Centralizing this avoids drift if truncation or wording changes again.♻️ Suggested refactor
+private fun throwClickHouseQueryError(operation: String, sql: String, body: String): Nothing { + logger.error { + "ClickHouse error in $operation. SQL: ${sql.take(LOG_SQL_MAX_LEN)} Body: ${body.take(LOG_BODY_MAX_LEN)}" + } + throw IllegalStateException("ClickHouse query error: ${body.take(LOG_BODY_MAX_LEN)}") +} + private suspend fun executeCount(sql: String): Long { val resp = ClickHouseClient.execute(sql) val body = resp.bodyAsText() if (resp.isClickHouseError(body)) { - logger.error { - "ClickHouse error in executeCount. SQL: ${sql.take(LOG_SQL_MAX_LEN)} Body: ${body.take(LOG_BODY_MAX_LEN)}" - } - throw IllegalStateException("ClickHouse query error: ${body.take(LOG_BODY_MAX_LEN)}") + throwClickHouseQueryError("executeCount", sql, body) } return body.trim().lines().firstOrNull()?.let { json.parseToJsonElement(it).jsonObject["cnt"] ?.jsonPrimitive?.content?.toLongOrNull() } ?: 0L } @@ private suspend fun executeRows( sql: String, mapper: (JsonObject) -> JsonObject ): List<JsonObject> { val resp = ClickHouseClient.execute(sql) val body = resp.bodyAsText() if (resp.isClickHouseError(body)) { - logger.error { - "ClickHouse error in executeRows. SQL: ${sql.take(LOG_SQL_MAX_LEN)} Body: ${body.take(LOG_BODY_MAX_LEN)}" - } - throw IllegalStateException("ClickHouse query error: ${body.take(LOG_BODY_MAX_LEN)}") + throwClickHouseQueryError("executeRows", sql, body) } return body.trim().lines().filter { it.isNotBlank() }.map { line -> mapper(json.parseToJsonElement(line).jsonObject) } }Also applies to: 263-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/security/SecurityQueryRoutes.kt` around lines 245 - 249, Both executeCount and executeRows duplicate ClickHouse error logging and exception creation; extract that logic into a single helper (e.g., handleClickHouseError) that accepts the SQL string, response body, and any contextual message or logger, uses LOG_SQL_MAX_LEN and LOG_BODY_MAX_LEN to truncate values, calls logger.error with the formatted message (including truncated SQL and body), and throws the IllegalStateException with the truncated body; then replace the duplicate blocks in executeCount and executeRows with a call to handleClickHouseError(sql, body, "ClickHouse query error") so truncation/wording is maintained in one place.backend/src/main/kotlin/com/moneat/dashboards/translation/DataDogTranslator.kt (1)
43-47: Constants are well-defined with clear naming.The
DD_prefix clearly scopes these as DataDog-specific. One minor observation:GrafanaTranslatorderivesMONEAT_MAX_COL_IDXfromMONEAT_COLS - 1, whereas hereDD_MAX_GRID_COL = 11is hardcoded separately fromDD_GRID_COLS = 12. Consider deriving it for consistency:private const val DD_MAX_GRID_COL = DD_GRID_COLS - 1This isn't blocking—both approaches satisfy the MagicNumber rule.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/translation/DataDogTranslator.kt` around lines 43 - 47, DD_MAX_GRID_COL is hardcoded to 11 while DD_GRID_COLS is 12; change DD_MAX_GRID_COL to be derived from DD_GRID_COLS (i.e., set DD_MAX_GRID_COL = DD_GRID_COLS - 1) so the relationship is explicit and consistent with GrafanaTranslator's pattern; update the constant declaration for DD_MAX_GRID_COL in DataDogTranslator to compute from DD_GRID_COLS and ensure any usages of DD_MAX_GRID_COL continue to work without other changes.ee/backend/src/main/kotlin/com/moneat/enterprise/sso/services/SsoService.kt (1)
607-608: Optional: derive signature index from part-count to avoid future drift.
SSO_STATE_SIGNATURE_IDXcan be computed fromSSO_STATE_PART_COUNTso these can’t diverge during future format edits.♻️ Proposed small refactor
- private const val SSO_STATE_PART_COUNT = 4 - private const val SSO_STATE_SIGNATURE_IDX = 3 + private const val SSO_STATE_PART_COUNT = 4 + private const val SSO_STATE_SIGNATURE_IDX = SSO_STATE_PART_COUNT - 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/src/main/kotlin/com/moneat/enterprise/sso/services/SsoService.kt` around lines 607 - 608, SSO_STATE_SIGNATURE_IDX should be derived from SSO_STATE_PART_COUNT to prevent drift: replace the separate constant SSO_STATE_SIGNATURE_IDX with a computed value (e.g. SSO_STATE_PART_COUNT - 1) wherever SSO_STATE_SIGNATURE_IDX is used in SsoService (file SsoService.kt) so the signature index always reflects the part count; update usages to reference the computed expression or a small helper (e.g. getSignatureIndex()) and remove the redundant constant.backend/src/main/kotlin/com/moneat/billing/services/StripeService.kt (1)
830-832: Consider extracting the repeated division to a named constant.The expression
BYTES_PER_GB / GB_METER_UNITS_PER_GBappears twice. Extracting it to a constant likeBYTES_PER_METER_UNITwould improve readability and make the intent clearer.♻️ Suggested refactor
In the companion object:
private const val BYTES_PER_METER_UNIT = BYTES_PER_GB / GB_METER_UNITS_PER_GBThen simplify the calculations:
-val drainableUnits = pendingBytes / (BYTES_PER_GB / GB_METER_UNITS_PER_GB) +val drainableUnits = pendingBytes / BYTES_PER_METER_UNIT if (drainableUnits > 0) { - val remainingBytes = pendingBytes - drainableUnits * (BYTES_PER_GB / GB_METER_UNITS_PER_GB) + val remainingBytes = pendingBytes - drainableUnits * BYTES_PER_METER_UNIT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/StripeService.kt` around lines 830 - 832, The repeated expression BYTES_PER_GB / GB_METER_UNITS_PER_GB in StripeService.kt (used when computing drainableUnits and remainingBytes) should be extracted to a named constant in the companion object (e.g., private const val BYTES_PER_METER_UNIT) and then used in the calculations for drainableUnits and remainingBytes to improve readability and avoid duplication; update the references in the code that compute drainableUnits and remainingBytes to use BYTES_PER_METER_UNIT instead of repeating the division.backend/src/main/kotlin/com/moneat/ai/OpenAiClient.kt (1)
46-49: Consider using Ktor's built-inHttpStatusCodeconstants.While the named constants satisfy the MagicNumber rule, Ktor provides these values as part of its API. Using
HttpStatusCode.Unauthorized.value,HttpStatusCode.TooManyRequests.value, and a range check againstHttpStatusCode.BadRequest.value..HttpStatusCode.UnavailableForLegalReasons.valuewould be more idiomatic and reduce custom constant declarations.♻️ Refactor to use Ktor constants
- private const val HTTP_UNAUTHORIZED = 401 - private const val HTTP_TOO_MANY_REQUESTS = 429 - private const val HTTP_CLIENT_ERROR_MIN = 400 - private const val HTTP_CLIENT_ERROR_MAX = 499 - private val json = Json { ignoreUnknownKeys = true }Then update the when statement:
throw when (response.status.value) { - HTTP_UNAUTHORIZED -> OpenAiError.AuthenticationError("Invalid or expired API key") - HTTP_TOO_MANY_REQUESTS -> OpenAiError.RateLimitError("Rate limit exceeded — please try again later") - in HTTP_CLIENT_ERROR_MIN..HTTP_CLIENT_ERROR_MAX -> + HttpStatusCode.Unauthorized.value -> OpenAiError.AuthenticationError("Invalid or expired API key") + HttpStatusCode.TooManyRequests.value -> OpenAiError.RateLimitError("Rate limit exceeded — please try again later") + in HttpStatusCode.BadRequest.value..HttpStatusCode.UnavailableForLegalReasons.value -> OpenAiError.ModelError("OpenAI request error (${response.status.value}): $body") else -> OpenAiError.ServerError("OpenAI server error (${response.status.value})") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/ai/OpenAiClient.kt` around lines 46 - 49, Replace the custom HTTP constants in OpenAiClient (HTTP_UNAUTHORIZED, HTTP_TOO_MANY_REQUESTS, HTTP_CLIENT_ERROR_MIN, HTTP_CLIENT_ERROR_MAX) with Ktor's HttpStatusCode constants: use HttpStatusCode.Unauthorized.value and HttpStatusCode.TooManyRequests.value where specific codes are needed, and perform range checks using HttpStatusCode.BadRequest.value..HttpStatusCode.UnavailableForLegalReasons.value instead of HTTP_CLIENT_ERROR_MIN/MAX; update the response handling when expression in the OpenAiClient class to use these HttpStatusCode values/range checks so the code is more idiomatic and removes the unused magic-number constants.backend/src/main/kotlin/com/moneat/datadog/services/DatadogMetricService.kt (2)
127-130: Replace@Suppress("MagicNumber")with named constant.The
@Suppress("MagicNumber")annotation on line 127 suppresses the magic number1000used for timestamp conversion on line 130. As per coding guidelines, when fixing a static analysis issue, you should fix the root cause instead of suppressing. Use the sameMILLIS_PER_SECONDconstant as suggested for theflattenV1Pointsfunction above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/DatadogMetricService.kt` around lines 127 - 130, Replace the `@Suppress`("MagicNumber") on the QueuedSketchEntry construction in DatadogMetricService with a named constant: remove the suppression and use the existing MILLIS_PER_SECOND constant (the same one used in flattenV1Points) when computing timestampMs for QueuedSketchEntry (i.e., timestampMs = dist.ts * MILLIS_PER_SECOND) so the magic number 1000 is eliminated and the code matches the established constant usage.
104-106: Replace@Suppress("MagicNumber")with named constant.The
@Suppress("MagicNumber")annotation on line 104 suppresses the magic number1000used for timestamp conversion on line 105. As per coding guidelines, when fixing a static analysis issue, you should fix the root cause instead of suppressing. Consider introducing a named constant (consistent with what should be added toDatadogEventService.kt):private const val MILLIS_PER_SECOND = 1000LThen replace line 105:
val timestampMs = (point[0] * MILLIS_PER_SECOND).toLong()This aligns with the PR objective to enable the MagicNumber rule and eliminate violations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/DatadogMetricService.kt` around lines 104 - 106, Replace the suppressed magic number by introducing a named constant (e.g., MILLIS_PER_SECOND) and use it in the timestamp conversion instead of multiplying by literal 1000; update DatadogMetricService (the computation that sets timestampMs from point[0]) to compute timestampMs = (point[0] * MILLIS_PER_SECOND).toLong(), matching the same constant pattern used in DatadogEventService to remove the `@Suppress`("MagicNumber") annotation.backend/src/main/kotlin/com/moneat/datadog/services/DatadogEventService.kt (2)
78-90: Replace@Suppress("MagicNumber")with named constant.The
@Suppress("MagicNumber")annotation on line 78 suppresses the magic number1000used for timestamp conversion on line 87. As per coding guidelines, when fixing a static analysis issue, you should fix the root cause instead of suppressing. Consider introducing a named constant:private const val MILLIS_PER_SECOND = 1000LThen replace line 87:
event.dateHappened * MILLIS_PER_SECONDThis aligns with the PR objective to enable the MagicNumber rule and eliminate violations rather than suppress them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/DatadogEventService.kt` around lines 78 - 90, Remove the `@Suppress`("MagicNumber") from DatadogEventService and introduce a named constant (e.g., private const val MILLIS_PER_SECOND = 1000L) near the top of the class, then update DatadogEventService.mapEvents to use event.dateHappened * MILLIS_PER_SECOND instead of the literal 1000; ensure the new constant is used for all timestamp-second-to-millis conversions in this class and delete the suppression annotation.
117-130: Replace@Suppress("MagicNumber")with named constant.The
@Suppress("MagicNumber")annotation on line 117 suppresses the magic number1000used for timestamp conversion on line 126. As per coding guidelines, when fixing a static analysis issue, you should fix the root cause instead of suppressing. Use the sameMILLIS_PER_SECONDconstant as suggested for themapEventsfunction above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/DatadogEventService.kt` around lines 117 - 130, Remove the `@Suppress`("MagicNumber") on mapServiceChecks and replace the hardcoded 1000 with the shared MILLIS_PER_SECOND constant used in mapEvents; update the timestamp conversion in DatadogEventService.mapServiceChecks to multiply sc.timestamp by MILLIS_PER_SECOND (keeping now fallback) so the magic number is eliminated and consistency with the existing constant is preserved (the change affects the timestampMs calculation where QueuedServiceCheckEntry is constructed; no other behavior changes).backend/src/main/kotlin/com/moneat/events/services/DashboardQueryHelper.kt (1)
376-379: Consider reformatting for readability.The line break mid-argument (
LOG_SNIPPET_CHARSon its own line insidetake()) is awkward. A cleaner approach would be to extract the truncated query to a local variable or format the entire message as a multi-line string.♻️ Suggested refactor
}.getOrElse { e -> - logger.error(e) { - "executeSlowestTransactionsQuery failed (transport/parse): query=${query.take( - LOG_SNIPPET_CHARS - )}, returning emptyList" - } + val snippet = query.take(LOG_SNIPPET_CHARS) + logger.error(e) { + "executeSlowestTransactionsQuery failed (transport/parse): query=$snippet, returning emptyList" + } emptyList() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/events/services/DashboardQueryHelper.kt` around lines 376 - 379, The logger.error call inside DashboardQueryHelper.executeSlowestTransactionsQuery is hard to read because the query.truncate call is split across lines; extract the truncated query into a local val (e.g., val querySnippet = query.take(LOG_SNIPPET_CHARS)) and use that variable in the logger.error message (or build a clear multi-line message) so the log invocation is a single, readable expression and avoids breaking the take(LOG_SNIPPET_CHARS) call across lines.backend/src/main/kotlin/com/moneat/utils/UrlValidator.kt (1)
155-159: Consider extracting metadata IP octets as constants.While array indices correctly use constants, the IP address octets (169, 254, 169, 254) remain as literals. For complete consistency with the MagicNumber rule, consider extracting them.
♻️ Optional: Extract IP octets as constants
+ private const val METADATA_IP_OCTET_0 = 169 + private const val METADATA_IP_OCTET_1 = 254 + private const val METADATA_IP_OCTET_2 = 169 + private const val METADATA_IP_OCTET_3 = 254 + private fun isMetadataAddress(addr: InetAddress): Boolean { val bytes = addr.address if (bytes.size != IPV4_BYTE_LENGTH) return false - return bytes[0] == 169.toByte() && - bytes[1] == 254.toByte() && - bytes[2] == 169.toByte() && + return bytes[0] == METADATA_IP_OCTET_0.toByte() && + bytes[1] == METADATA_IP_OCTET_1.toByte() && + bytes[2] == METADATA_IP_OCTET_2.toByte() && - bytes[IPV4_LAST_BYTE_IDX] == 254.toByte() + bytes[IPV4_LAST_BYTE_IDX] == METADATA_IP_OCTET_3.toByte() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/utils/UrlValidator.kt` around lines 155 - 159, The code in UrlValidator.kt uses literal octet values (169, 254, 169, 254) when checking the bytes array; extract these magic numbers into clearly named constants (e.g., IPV4_LINK_LOCAL_OCTET_0, IPV4_LINK_LOCAL_OCTET_1, etc.) alongside existing constants like IPV4_BYTE_LENGTH and IPV4_LAST_BYTE_IDX, then replace the literals in the bytes[...] comparisons inside the method that validates link-local IPv4 addresses so the checks read bytes[0] == IPV4_LINK_LOCAL_OCTET_0 && bytes[1] == IPV4_LINK_LOCAL_OCTET_1 ... for consistency with the MagicNumber rule.backend/src/main/kotlin/com/moneat/datadog/services/ProfileIngestionService.kt (1)
365-371: LGTM! Consider a constant for the fallback threshold.The use of
MIN_PROFILE_PARTSimproves clarity. For consistency, the literal2at line 367 could also be extracted to a constant likeMIN_PROFILE_META_FALLBACK_PARTS = 2, which would document that the fallback case requires at least the storage key and profile type.♻️ Optional: Extract fallback threshold constant for consistency
private const val NANOS_PER_MILLI = 1_000_000L private const val MIN_PROFILE_PARTS = 3 +private const val MIN_PROFILE_META_FALLBACK_PARTS = 2 ... return if (parts.size >= MIN_PROFILE_PARTS) { ProfileMeta(parts[0], parts[1], parts[2]) -} else if (parts.size >= 2) { +} else if (parts.size >= MIN_PROFILE_META_FALLBACK_PARTS) { ProfileMeta(parts[0], parts[1], "datadog") } else { null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/ProfileIngestionService.kt` around lines 365 - 371, Extract the magic literal 2 into a named constant for consistency with MIN_PROFILE_PARTS: add a constant (e.g. MIN_PROFILE_META_FALLBACK_PARTS = 2) near the other constants and replace the literal `2` in the conditional that builds ProfileMeta(parts[0], parts[1], "datadog") so the code reads `else if (parts.size >= MIN_PROFILE_META_FALLBACK_PARTS)`, keeping the existing behavior but documenting the fallback threshold.backend/src/main/kotlin/com/moneat/shared/services/RetentionPolicyService.kt (1)
32-35: PreferPricingTier.FREEover duplicating retention defaults.Lines 34-35 create a second copy of plan-retention values that already live on
PricingTier. If the free-tier policy changes, the exception fallback path here will drift silently.♻️ Proposed fix
companion object { const val RETENTION_CACHE_TTL_SECONDS = 300L - private const val FREE_TIER_LOG_RETENTION_DAYS = 3 - private const val DEFAULT_ANALYTICS_RETENTION_DAYS = 1095 } @@ logRetentionByOrg[orgId] = runCatching { getLogRetentionDaysForOrganization(orgId) - }.getOrDefault(FREE_TIER_LOG_RETENTION_DAYS) // Default to FREE tier log retention + }.getOrDefault(PricingTier.FREE.logRetentionDays) @@ analyticsRetentionByOrg[orgId] = runCatching { getAnalyticsRetentionDaysForOrganization(orgId) } - .getOrDefault(DEFAULT_ANALYTICS_RETENTION_DAYS) // Default 3 years for analytics + .getOrDefault(PricingTier.FREE.analyticsRetentionDays)Also applies to: 106-115, 162-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/shared/services/RetentionPolicyService.kt` around lines 32 - 35, The companion object duplicates free-tier retention defaults (FREE_TIER_LOG_RETENTION_DAYS, DEFAULT_ANALYTICS_RETENTION_DAYS) that already exist on PricingTier; remove these duplicated constants and switch the fallback/exception paths in RetentionPolicyService to read the values from PricingTier.FREE (e.g., PricingTier.FREE.logRetentionDays and PricingTier.FREE.analyticsRetentionDays or the equivalent properties/methods on PricingTier) wherever those constants are used (including the other duplicate sites referenced around the get/fallback logic).backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt (1)
354-364: Consider consistency in index constant usage.Indices 0, 1, 2 are still used as literals (lines 355-357) while indices 3-8 use constants. This inconsistency is minor since low indices (0, 1, 2) are typically self-documenting, but for full consistency you could add
CLUSTER_NODE_ID_IDX = 0,CLUSTER_NODE_ADDRESS_IDX = 1,CLUSTER_NODE_FLAGS_IDX = 2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt` around lines 354 - 364, The map construction mixes literal indices (0,1,2) with named constants (CLUSTER_NODE_MASTER_IDX, etc.); add constants CLUSTER_NODE_ID_IDX = 0, CLUSTER_NODE_ADDRESS_IDX = 1 and CLUSTER_NODE_FLAGS_IDX = 2 and replace the literal usages in the JsonPrimitive calls (the parts.getOrElse(...) calls that currently use 0, 1, 2) so all indices use the same named-constant pattern as CLUSTER_NODE_MASTER_IDX, CLUSTER_NODE_PING_IDX, CLUSTER_NODE_PONG_IDX, CLUSTER_NODE_EPOCH_IDX, CLUSTER_NODE_LINK_IDX, and CLUSTER_NODE_SLOT_IDX.ee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.kt (1)
409-416: UseMonth.entriesinstead ofMonth.values()for better performance and idiomatic Kotlin.Since Kotlin 1.9.0,
Month.entriesis the recommended way to iterate enum constants. It returns an immutable list without allocating a new array on each call, unlikevalues().♻️ Proposed fix
- private val MONTH_NUMBERS: Map<String, Int> = Month.values().flatMap { m -> + private val MONTH_NUMBERS: Map<String, Int> = Month.entries.flatMap { m -> val name = m.name.lowercase() if (name.length > 3) { listOf(name to m.value, name.take(3) to m.value) } else { listOf(name to m.value) } }.toMap()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.kt` around lines 409 - 416, Replace the use of Month.values() when building MONTH_NUMBERS with Month.entries to avoid the array allocation and follow Kotlin idioms; update the expression in the MONTH_NUMBERS initializer so it iterates Month.entries and retains the same mapping logic that emits full lowercase names and 3-letter keys mapped to m.value (keep the same branching on name.length > 3 and the toMap() call).backend/src/main/kotlin/com/moneat/datadog/services/TraceIngestionService.kt (1)
60-73: Constants for protobuf parsing look good, but fields 1 and 2 are missing.Fields 3–14 have named constants, but fields 1 and 2 are still used as inline literals (e.g., lines 771, 836, 872). For consistency, consider adding
PROTO_FIELD_1andPROTO_FIELD_2.♻️ Suggested addition
private const val PROTO_TAG_SHIFT = 3 +private const val PROTO_FIELD_1 = 1 +private const val PROTO_FIELD_2 = 2 private const val PROTO_FIELD_3 = 3Then update usages like
(1 shl PROTO_TAG_SHIFT)to(PROTO_FIELD_1 shl PROTO_TAG_SHIFT).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/TraceIngestionService.kt` around lines 60 - 73, Add missing protobuf field constants PROTO_FIELD_1 and PROTO_FIELD_2 alongside the existing constants (e.g., in TraceIngestionService.kt where HEX_RADIX and PROTO_FIELD_3..14 are declared), then replace inline literals used as field numbers (for example occurrences of (1 shl PROTO_TAG_SHIFT) and (2 shl PROTO_TAG_SHIFT) in the protobuf parsing logic) with the named constants (PROTO_FIELD_1 shl PROTO_TAG_SHIFT and PROTO_FIELD_2 shl PROTO_TAG_SHIFT) so all field references are consistent and self-documenting.backend/src/main/kotlin/com/moneat/dashboards/services/handlers/JdbcHandler.kt (1)
41-51: Good constant definitions, but two are unused.
POOL_MIN_IDLEandTEMP_POOL_MAX_SIZEare defined but not referenced. Lines 166 and 188 still use hardcoded literals0and1respectively.Proposed fix to use the defined constants
protected open fun createPool( ... ): HikariDataSource { val config = HikariConfig().apply { ... maximumPoolSize = POOL_MAX_SIZE - minimumIdle = 0 + minimumIdle = POOL_MIN_IDLE idleTimeout = POOL_IDLE_TIMEOUT_MS ... } return HikariDataSource(config) } private fun createTempDataSource( ... ): HikariDataSource { val config = HikariConfig().apply { ... - maximumPoolSize = 1 + maximumPoolSize = TEMP_POOL_MAX_SIZE connectionTimeout = POOL_CONNECTION_TIMEOUT_MS ... } return HikariDataSource(config) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/JdbcHandler.kt` around lines 41 - 51, Constants POOL_MIN_IDLE and TEMP_POOL_MAX_SIZE are defined in JdbcHandler's companion object but not used; replace the hardcoded literals currently used in the pool configuration with these constants: in the code that builds the main HikariPool (where a `0` is used) swap that literal for POOL_MIN_IDLE, and in the temporary pool creation (where a `1` is used) swap that literal for TEMP_POOL_MAX_SIZE so the companion-object constants POOL_MIN_IDLE and TEMP_POOL_MAX_SIZE are actually referenced.backend/src/main/kotlin/com/moneat/dashboards/routes/DashboardRoutes.kt (1)
423-424: Keep the batch-limit error text in sync with the constant.Line 424 still embeds
10directly, so future updates toMAX_QUERIES_PER_REQUESTcan desync behavior vs. message.♻️ Suggested tweak
- call.respond(HttpStatusCode.BadRequest, ErrorResponse("Maximum 10 queries per batch")) + call.respond( + HttpStatusCode.BadRequest, + ErrorResponse("Maximum $MAX_QUERIES_PER_REQUEST queries per batch") + )As per coding guidelines "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/routes/DashboardRoutes.kt` around lines 423 - 424, The error message embeds the magic number "10" instead of using the MAX_QUERIES_PER_REQUEST constant; update the call.respond(ErrorResponse(...)) in the request size check so the message uses the value of MAX_QUERIES_PER_REQUEST (e.g., interpolate or build the string from that constant) — locate the conditional that checks request.queries.size > MAX_QUERIES_PER_REQUEST and change the ErrorResponse text passed to call.respond to reference MAX_QUERIES_PER_REQUEST rather than the hardcoded 10.backend/src/main/kotlin/com/moneat/uptime/routes/UptimeRoutes.kt (1)
528-529: Optional: keep validation message tied toMIN_INTERVAL_SECONDS.Line 529 hardcodes
10, which can drift from the constant over time.♻️ Suggested tweak
- return "Check interval must be at least 10 seconds" + return "Check interval must be at least $MIN_INTERVAL_SECONDS seconds"As per coding guidelines "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/uptime/routes/UptimeRoutes.kt` around lines 528 - 529, The validation message currently hardcodes "10" instead of using the MIN_INTERVAL_SECONDS constant; update the response returned by the check in UptimeRoutes (the block comparing request.intervalSeconds to MIN_INTERVAL_SECONDS) to reference MIN_INTERVAL_SECONDS (e.g., interpolate or format the constant into the string) so the message stays in sync with the constant's value.backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsRoutes.kt (1)
99-107: UsesuspendRunCatchinginstead ofrunCatchingin this suspend function.The function is
suspend, but it usesrunCatchingwhich does not preserveCancellationExceptionbehavior. Switch tosuspendRunCatchingfromcom.moneat.utils.♻️ Proposed fix
+import com.moneat.utils.suspendRunCatching ... private suspend fun executeChQuery(query: String): List<JsonObject>? { - return runCatching { + return suspendRunCatching { val response = ClickHouseClient.execute(query) if (response.status.value !in HTTP_SUCCESS_MIN..HTTP_SUCCESS_MAX) { logger.warn { "ClickHouse query failed: ${response.status}" } return null } parseJsonEachRow(response.bodyAsText()) }.getOrElse { logger.warn { "ClickHouse query error: ${it.message}" } null } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsRoutes.kt` around lines 99 - 107, The suspend function executeChQuery currently uses runCatching which can suppress CancellationException; replace runCatching with suspendRunCatching (from com.moneat.utils) so coroutine cancellation is propagated correctly, i.e., call suspendRunCatching { ... } around the ClickHouseClient.execute/query parsing block and add the appropriate import for suspendRunCatching.backend/src/main/kotlin/com/moneat/uptime/services/UptimeService.kt (1)
170-170: Avoid Double round-trip for heartbeat timestamps.Line 170/178 converts ms → seconds(Double) → ms. Keep it in milliseconds throughout to avoid unnecessary floating-point conversion.
♻️ Proposed refactor
- val timestamp = Clock.System.now().toEpochMilliseconds() / MILLIS_PER_SECOND.toDouble() + val timestampMs = Clock.System.now().toEpochMilliseconds() ... - fromUnixTimestamp64Milli(${(timestamp * MILLIS_PER_SECOND).toLong()}), + fromUnixTimestamp64Milli($timestampMs),Also applies to: 178-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/uptime/services/UptimeService.kt` at line 170, The heartbeat timestamp is converted to seconds as a Double then later implicitly treated as milliseconds, causing an unnecessary float round-trip; change the timestamp calculation in UptimeService to keep milliseconds by using Clock.System.now().toEpochMilliseconds() (remove the division by MILLIS_PER_SECOND.toDouble()), and make the same change for the other occurrence referenced; update any downstream uses/fields that expect seconds to accept milliseconds (or convert once at the final consumer) and ensure constants (MILLIS_PER_SECOND) are no longer used for this local calculation.backend/src/main/kotlin/com/moneat/org/services/AdminService.kt (1)
657-661: Keep the storage alert copy derived from the threshold constants.The comparisons now use
STORAGE_WARNING_THRESHOLD_PCTandSTORAGE_CRITICAL_THRESHOLD_PCT, but the messages still hardcode70%and80%. If those thresholds change, the alert text will immediately drift.♻️ Proposed fix
if (storageUsedPercent > STORAGE_WARNING_THRESHOLD_PCT) { - alerts.add("Storage > 70% (consider adding block storage)") + alerts.add( + "Storage > ${STORAGE_WARNING_THRESHOLD_PCT.toInt()}% (consider adding block storage)" + ) } if (storageUsedPercent > STORAGE_CRITICAL_THRESHOLD_PCT) { - alerts.add("Storage > 80% (scaling trigger)") + alerts.add("Storage > ${STORAGE_CRITICAL_THRESHOLD_PCT.toInt()}% (scaling trigger)") }As per coding guidelines, "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt` around lines 657 - 661, The alert messages hardcode "70%" and "80%" while the checks use STORAGE_WARNING_THRESHOLD_PCT and STORAGE_CRITICAL_THRESHOLD_PCT; update the alerts added via alerts.add(...) to interpolate or format the threshold constant values (STORAGE_WARNING_THRESHOLD_PCT and STORAGE_CRITICAL_THRESHOLD_PCT) into the message string so the text always reflects the actual constants used in the comparisons (e.g., include the constant value followed by "%" in the descriptive message).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc4884b5-bd91-4cc5-a455-226a0e2e9741
📒 Files selected for processing (138)
.line-len-check.jsbackend/detekt.ymlbackend/src/main/kotlin/com/moneat/Application.ktbackend/src/main/kotlin/com/moneat/ai/AiChatService.ktbackend/src/main/kotlin/com/moneat/ai/OpenAiClient.ktbackend/src/main/kotlin/com/moneat/analytics/routes/AnalyticsRoutes.ktbackend/src/main/kotlin/com/moneat/analytics/services/AnalyticsIngestionWorker.ktbackend/src/main/kotlin/com/moneat/analytics/services/AnalyticsService.ktbackend/src/main/kotlin/com/moneat/auth/routes/AuthRoutes.ktbackend/src/main/kotlin/com/moneat/auth/services/AuthService.ktbackend/src/main/kotlin/com/moneat/auth/services/AuthTokenService.ktbackend/src/main/kotlin/com/moneat/auth/services/OAuthService.ktbackend/src/main/kotlin/com/moneat/auth/services/RefreshTokenService.ktbackend/src/main/kotlin/com/moneat/billing/routes/BillingRoutes.ktbackend/src/main/kotlin/com/moneat/billing/services/BillingBackgroundService.ktbackend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.ktbackend/src/main/kotlin/com/moneat/billing/services/PricingTierService.ktbackend/src/main/kotlin/com/moneat/billing/services/StripeService.ktbackend/src/main/kotlin/com/moneat/config/ClickHouseClient.ktbackend/src/main/kotlin/com/moneat/config/ClickHouseMigrations.ktbackend/src/main/kotlin/com/moneat/config/SentryConfig.ktbackend/src/main/kotlin/com/moneat/dashboards/repositories/DashboardRepositoryImpl.ktbackend/src/main/kotlin/com/moneat/dashboards/repositories/DashboardWidgetRepositoryImpl.ktbackend/src/main/kotlin/com/moneat/dashboards/routes/DashboardRoutes.ktbackend/src/main/kotlin/com/moneat/dashboards/services/DashboardAlertService.ktbackend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/BigQueryHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/CloudWatchHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/ElasticsearchHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/GraphiteHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/InfluxDBHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/JdbcHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/LokiHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/MongoDBHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/PrometheusHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/translation/DataDogTranslator.ktbackend/src/main/kotlin/com/moneat/dashboards/translation/GrafanaTranslator.ktbackend/src/main/kotlin/com/moneat/datadog/decompression/DecompressionService.ktbackend/src/main/kotlin/com/moneat/datadog/decompression/MetricPayloadDecoder.ktbackend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.ktbackend/src/main/kotlin/com/moneat/datadog/routes/DatadogDogStatsDRoutes.ktbackend/src/main/kotlin/com/moneat/datadog/routes/DatadogHostRoutes.ktbackend/src/main/kotlin/com/moneat/datadog/routes/DatadogInfraQueryRoutes.ktbackend/src/main/kotlin/com/moneat/datadog/security/SecurityQueryRoutes.ktbackend/src/main/kotlin/com/moneat/datadog/services/DatadogEventService.ktbackend/src/main/kotlin/com/moneat/datadog/services/DatadogInfraService.ktbackend/src/main/kotlin/com/moneat/datadog/services/DatadogJfrFlamegraphService.ktbackend/src/main/kotlin/com/moneat/datadog/services/DatadogMetricService.ktbackend/src/main/kotlin/com/moneat/datadog/services/DatadogPprofFlamegraphService.ktbackend/src/main/kotlin/com/moneat/datadog/services/ProfileIngestionService.ktbackend/src/main/kotlin/com/moneat/datadog/services/ProfileStorageService.ktbackend/src/main/kotlin/com/moneat/datadog/services/TraceIngestionService.ktbackend/src/main/kotlin/com/moneat/enterprise/license/LicenseValidator.ktbackend/src/main/kotlin/com/moneat/events/models/SentryModels.ktbackend/src/main/kotlin/com/moneat/events/repositories/ProjectRepositoryImpl.ktbackend/src/main/kotlin/com/moneat/events/routes/ApiRoutes.ktbackend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.ktbackend/src/main/kotlin/com/moneat/events/services/AccessService.ktbackend/src/main/kotlin/com/moneat/events/services/DashboardQueryHelper.ktbackend/src/main/kotlin/com/moneat/events/services/EventService.ktbackend/src/main/kotlin/com/moneat/events/services/IngestionWorker.ktbackend/src/main/kotlin/com/moneat/events/services/IssueService.ktbackend/src/main/kotlin/com/moneat/events/services/ProjectService.ktbackend/src/main/kotlin/com/moneat/events/services/ProjectStatsService.ktbackend/src/main/kotlin/com/moneat/events/services/ReleaseService.ktbackend/src/main/kotlin/com/moneat/events/services/ReleaseStatsService.ktbackend/src/main/kotlin/com/moneat/events/services/ReplayService.ktbackend/src/main/kotlin/com/moneat/events/services/SentrySpanBackfill.ktbackend/src/main/kotlin/com/moneat/events/services/TransactionService.ktbackend/src/main/kotlin/com/moneat/incident/routes/IncidentProviderRoutes.ktbackend/src/main/kotlin/com/moneat/incident/services/IncidentIoProvider.ktbackend/src/main/kotlin/com/moneat/llm/routes/LlmRoutes.ktbackend/src/main/kotlin/com/moneat/llm/services/LlmDashboardService.ktbackend/src/main/kotlin/com/moneat/llm/services/LlmIngestionWorker.ktbackend/src/main/kotlin/com/moneat/logging/MoneatLogAppender.ktbackend/src/main/kotlin/com/moneat/logs/routes/LogRoutes.ktbackend/src/main/kotlin/com/moneat/logs/services/LogService.ktbackend/src/main/kotlin/com/moneat/monitor/routes/InfraRoutes.ktbackend/src/main/kotlin/com/moneat/monitor/routes/MonitorRoutes.ktbackend/src/main/kotlin/com/moneat/monitor/services/MonitorAlertService.ktbackend/src/main/kotlin/com/moneat/monitor/services/MonitorService.ktbackend/src/main/kotlin/com/moneat/notifications/services/DiscordService.ktbackend/src/main/kotlin/com/moneat/notifications/services/EmailService.ktbackend/src/main/kotlin/com/moneat/notifications/services/NotificationService.ktbackend/src/main/kotlin/com/moneat/notifications/services/SlackService.ktbackend/src/main/kotlin/com/moneat/org/routes/AdminRoutes.ktbackend/src/main/kotlin/com/moneat/org/routes/IntegrationRoutes.ktbackend/src/main/kotlin/com/moneat/org/services/AdminService.ktbackend/src/main/kotlin/com/moneat/org/services/OrgInvitationService.ktbackend/src/main/kotlin/com/moneat/otlp/services/OtlpMetricsService.ktbackend/src/main/kotlin/com/moneat/otlp/services/OtlpTraceService.ktbackend/src/main/kotlin/com/moneat/plugins/BackgroundJobs.ktbackend/src/main/kotlin/com/moneat/plugins/Databases.ktbackend/src/main/kotlin/com/moneat/plugins/Monitoring.ktbackend/src/main/kotlin/com/moneat/plugins/RateLimiting.ktbackend/src/main/kotlin/com/moneat/security/routes/SecurityRoutes.ktbackend/src/main/kotlin/com/moneat/shared/services/AnalyticsSaltService.ktbackend/src/main/kotlin/com/moneat/shared/services/ArtifactCleanupService.ktbackend/src/main/kotlin/com/moneat/shared/services/AttributionAnalyticsService.ktbackend/src/main/kotlin/com/moneat/shared/services/PulseService.ktbackend/src/main/kotlin/com/moneat/shared/services/RetentionBackgroundService.ktbackend/src/main/kotlin/com/moneat/shared/services/RetentionPolicyService.ktbackend/src/main/kotlin/com/moneat/shared/services/SdkVersionService.ktbackend/src/main/kotlin/com/moneat/shared/services/UsageTrackingService.ktbackend/src/main/kotlin/com/moneat/statuspage/services/StatusPageService.ktbackend/src/main/kotlin/com/moneat/summary/services/SummaryService.ktbackend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsCheckExecutor.ktbackend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsRoutes.ktbackend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsScheduler.ktbackend/src/main/kotlin/com/moneat/synthetics/routes/SyntheticsService.ktbackend/src/main/kotlin/com/moneat/uptime/routes/UptimeRoutes.ktbackend/src/main/kotlin/com/moneat/uptime/services/UptimeCheckExecutor.ktbackend/src/main/kotlin/com/moneat/uptime/services/UptimeScheduler.ktbackend/src/main/kotlin/com/moneat/uptime/services/UptimeService.ktbackend/src/main/kotlin/com/moneat/utils/ClickHouseQueryUtils.ktbackend/src/main/kotlin/com/moneat/utils/DemoClock.ktbackend/src/main/kotlin/com/moneat/utils/UrlValidator.ktee/backend/detekt.ymlee/backend/src/main/kotlin/com/moneat/enterprise/ai/llm/costs/Calculators.ktee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/AiContextAggregator.ktee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/services/SummaryService.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/tools/DashboardTemplateTool.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/tools/DataSourceTool.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/tools/MetricsTool.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/tools/MonitorTool.ktee/backend/src/main/kotlin/com/moneat/enterprise/mcp/tools/UptimeTool.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/IncidentRoutes.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/NotificationPreferencesRoutes.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/OnCallRoutes.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/BusinessHoursService.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/OnCallHandoffService.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/OnCallScheduleService.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/PushNotificationService.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/ShiftChangeNotifier.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/services/SlackUserGroupSyncService.ktee/backend/src/main/kotlin/com/moneat/enterprise/sso/services/SsoService.kt
backend/src/main/kotlin/com/moneat/analytics/services/AnalyticsService.kt
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/events/services/TransactionService.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/incident/routes/IncidentProviderRoutes.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/org/services/AdminService.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/src/main/kotlin/com/moneat/org/services/AdminService.kt (1)
657-662: Consider using the threshold constants in alert messages.The alert messages hardcode "70%" and "80%" while the thresholds are defined as constants. If thresholds change, the messages would need separate updates.
♻️ Optional: interpolate threshold values into messages
if (storageUsedPercent > STORAGE_WARNING_THRESHOLD_PCT) { - alerts.add("Storage > 70% (consider adding block storage)") + alerts.add("Storage > ${STORAGE_WARNING_THRESHOLD_PCT.toInt()}% (consider adding block storage)") } if (storageUsedPercent > STORAGE_CRITICAL_THRESHOLD_PCT) { - alerts.add("Storage > 80% (scaling trigger)") + alerts.add("Storage > ${STORAGE_CRITICAL_THRESHOLD_PCT.toInt()}% (scaling trigger)") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt` around lines 657 - 662, The alert messages currently hardcode "70%" and "80%"; update the calls to alerts.add inside the storage checks to interpolate or format the constants STORAGE_WARNING_THRESHOLD_PCT and STORAGE_CRITICAL_THRESHOLD_PCT (used with storageUsedPercent checks) into the message strings so the text reflects the actual threshold values; locate the two places where storageUsedPercent is compared and alerts.add is called and replace the literal percentages with formatted uses of the constants.backend/src/main/kotlin/com/moneat/logs/services/LogService.kt (1)
855-879: Repetitive catch blocks could be consolidated.The four catch blocks have identical logging logic. While this structure predates this PR, consider consolidating using
suspendRunCatchingor a multi-catch approach in a follow-up.♻️ Optional: Consolidate catch blocks
if (!query.isNullOrBlank()) { - // Use Datadog-compatible query parser - try { + suspendRunCatching { val parsed = queryParser.parse(query) if (parsed.rootNode != null) { val queryCondition = queryParser.toClickHouseSql(parsed.rootNode, ::escapeSql) if (queryCondition.isNotBlank() && queryCondition != "1=1") { conditions += "($queryCondition)" } } - } catch (e: SerializationException) { - logger.error(e) { - "Failed to parse query (query_fp=${utf8Fingerprint(query)}), falling back to simple search" - } - // Fallback: treat as simple full-text search - conditions += buildSimpleSearchCondition(query) - } catch (e: IOException) { - // ... repeated blocks - } + }.getOrElse { e -> + logger.error(e) { + "Failed to parse query (query_fp=${utf8Fingerprint(query)}), falling back to simple search" + } + conditions += buildSimpleSearchCondition(query) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt` around lines 855 - 879, Consolidate the four identical catch blocks in LogService.kt that handle SerializationException, IOException, IllegalStateException, and IllegalArgumentException into a single handler: wrap the parse logic (the try that generates conditions) with a runCatching/suspendRunCatching or use a multi-type catch so one catch handles all these exception types, and inside that single handler call logger.error(e) with the same message including utf8Fingerprint(query) and then append the fallback via conditions += buildSimpleSearchCondition(query); ensure you target the try surrounding the parsing logic (the block that currently logs and falls back) so behavior and message remain identical but without repeated catch blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt`:
- Around line 263-264: Remove the two unused constants DAYS_IN_WEEK and
DAYS_IN_MONTH from AdminService.kt; locate the declarations of private const val
DAYS_IN_WEEK and private const val DAYS_IN_MONTH and delete them, since the code
now uses PERIOD_7D_DAYS_BACK and PERIOD_30D_DAYS_BACK instead and these two
constants are unused.
---
Nitpick comments:
In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt`:
- Around line 855-879: Consolidate the four identical catch blocks in
LogService.kt that handle SerializationException, IOException,
IllegalStateException, and IllegalArgumentException into a single handler: wrap
the parse logic (the try that generates conditions) with a
runCatching/suspendRunCatching or use a multi-type catch so one catch handles
all these exception types, and inside that single handler call logger.error(e)
with the same message including utf8Fingerprint(query) and then append the
fallback via conditions += buildSimpleSearchCondition(query); ensure you target
the try surrounding the parsing logic (the block that currently logs and falls
back) so behavior and message remain identical but without repeated catch
blocks.
In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt`:
- Around line 657-662: The alert messages currently hardcode "70%" and "80%";
update the calls to alerts.add inside the storage checks to interpolate or
format the constants STORAGE_WARNING_THRESHOLD_PCT and
STORAGE_CRITICAL_THRESHOLD_PCT (used with storageUsedPercent checks) into the
message strings so the text reflects the actual threshold values; locate the two
places where storageUsedPercent is compared and alerts.add is called and replace
the literal percentages with formatted uses of the constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f98e3d17-a99a-4bd4-908f-a20b39e08ab6
📒 Files selected for processing (10)
backend/detekt.ymlbackend/src/main/kotlin/com/moneat/analytics/services/AnalyticsService.ktbackend/src/main/kotlin/com/moneat/auth/services/OAuthService.ktbackend/src/main/kotlin/com/moneat/config/DemoDataReseeder.ktbackend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.ktbackend/src/main/kotlin/com/moneat/events/services/TransactionService.ktbackend/src/main/kotlin/com/moneat/incident/routes/IncidentProviderRoutes.ktbackend/src/main/kotlin/com/moneat/logs/services/LogService.ktbackend/src/main/kotlin/com/moneat/org/services/AdminService.ktee/backend/detekt.yml
✅ Files skipped from review due to trivial changes (3)
- backend/detekt.yml
- backend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.kt
- backend/src/main/kotlin/com/moneat/config/DemoDataReseeder.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- ee/backend/detekt.yml
- backend/src/main/kotlin/com/moneat/incident/routes/IncidentProviderRoutes.kt
- backend/src/main/kotlin/com/moneat/auth/services/OAuthService.kt
- backend/src/main/kotlin/com/moneat/analytics/services/AnalyticsService.kt
backend/src/main/kotlin/com/moneat/org/services/AdminService.kt
Outdated
Show resolved
Hide resolved
SummaryService: add SECONDS_PER_MINUTE, MILLIS_PER_SECOND/HOUR/DAY, MILLIS_7_DAYS, MILLIS_30_DAYS; simplify periodToMillis() and use SECONDS_PER_MINUTE in metrics window calculation. LlmIngestionWorker: add HEADER_BYTE_INDEX_3/4/5/6 alongside the existing HEADER_LAST_BYTE_INDEX to name all non-trivial byte indices in the big-endian project-ID header decode/encode. BillingBackgroundService: add BILLING_JOB_INTERVAL_MS (60 s), QUOTA_WARNING_THRESHOLD (0.8), and CENTS_PER_DOLLAR (100.0). AuthService: add MIN_PASSWORD_LENGTH, ORG_SLUG_RANDOM_SUFFIX_LENGTH, JWT_ACCESS_TOKEN_EXPIRY_MS, DEMO_TOKEN_EXPIRY_MS, TOKEN_ENTROPY_BYTES, and MAX_ORG_SLUG_LENGTH to the companion object. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable MagicNumber in backend/detekt.yml and ee/backend/detekt.yml, and replace all remaining magic literals with named constants across ~130 files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove obsolete .line-len-check.js. Drop DemoDataReseeder from MagicNumber excludes in detekt (ee mirror) and suppress file-level MagicNumber for fixture-heavy demo seeding. Add DEFAULT_LIMIT for analytics defaults, HTTP success range for GitHub OAuth, safe envelope and SQL fingerprints in logs, split APDEX countIf SQL line, validate incident event log limit, align admin usage/email windows with PERIOD_*_DAYS_BACK, and redact ClickHouse export errors. Made-with: Cursor
AdrianLeeElder
force-pushed
the
fix/issue-279
branch
from
5254e2e to
382f2d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt (4)
321-335:⚠️ Potential issue | 🟡 Minor
@Suppress("MagicNumber")ondecodeCpuStat— same issue.Hardcoded field numbers (2, 5) should be extracted as constants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt` around lines 321 - 335, Extract the hardcoded protobuf field numbers in decodeCpuStat into named constants (e.g. CPU_TOTAL_PCT_FIELD = 2 and CPU_NUM_THREADS_FIELD = 5) and replace occurrences of (2 shl 3) or 5 and (5 shl 3) or 0 with expressions that use those constants (e.g. (CPU_TOTAL_PCT_FIELD shl 3) or 5 or appropriate tag builder constants) so the magic numbers are centralized; update the `@Suppress`("MagicNumber") usage accordingly and keep the function signature decodeCpuStat(bytes: ByteArray): Pair<Float, Int> unchanged.
136-165:⚠️ Potential issue | 🟡 Minor
@Suppress("MagicNumber")used instead of fixing root cause.The
decodeContainermethod suppresses MagicNumber instead of extracting constants for field numbers (2, 3, 4, 6, 8, 16, 17, 20, 21, 26). This contradicts the coding guidelines and the PR's goal of enabling the MagicNumber rule. The same pattern was successfully applied indecodeCollectorContainerand other methods.🔧 Proposed approach: Extract constants like other methods
Define constants for the frequently used field numbers (similar to
PROTO_FIELD_3,PROTO_FIELD_4, etc. in other files) and update thewhenbranches accordingly, then remove the@Suppressannotation.As per coding guidelines: "When fixing a compiler warning, linter finding, or static analysis issue (Detekt, ESLint, Sonar, Kotlin compiler), fix the root cause instead of suppressing with
@Suppress,eslint-disable,// NOSONAR, or similar".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt` around lines 136 - 165, The decodeContainer function currently uses magic numbers for protobuf field tags and suppresses MagicNumber; instead, define named constants for each field tag (e.g., PROTO_FIELD_ID/PROTO_FIELD_2, PROTO_FIELD_NAME/PROTO_FIELD_3, PROTO_FIELD_IMAGE/PROTO_FIELD_4, PROTO_FIELD_MEM_LIMIT/PROTO_FIELD_6, PROTO_FIELD_STATE/PROTO_FIELD_8, PROTO_FIELD_NET_RCVD/PROTO_FIELD_16, PROTO_FIELD_NET_SENT/PROTO_FIELD_17, PROTO_FIELD_TOTAL_PCT/PROTO_FIELD_20, PROTO_FIELD_MEM_RSS/PROTO_FIELD_21, PROTO_FIELD_TAGS/PROTO_FIELD_26) matching the pattern used in decodeCollectorContainer and other decoders, replace the numeric literals in the when branches with those constants, and then remove the `@Suppress`("MagicNumber") annotation.
211-257:⚠️ Potential issue | 🟡 Minor
@Suppress("MagicNumber")ondecodeProcess— same issue.This method also uses
@Suppress("MagicNumber")with hardcoded field numbers (2, 4, 5, 7, 8, 11, 12, 23) instead of extracting constants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt` around lines 211 - 257, The decodeProcess method currently suppresses MagicNumber and uses hardcoded protobuf tag numbers (e.g., (2 shl 3) or 0, (4 shl 3) or 2, etc.); replace those literal tag expressions with descriptive constants (e.g., TAG_PID, TAG_COMMAND, TAG_USER, TAG_MEMORY, TAG_CPU, TAG_OPEN_FD, TAG_STATE, TAG_TAGS) defined in a companion object or top-level area of ProcessAgentPayloadDecoder, update the when branches to use those named constants, remove the `@Suppress`("MagicNumber") annotation, and keep the existing logic calling decodeCommand, decodeProcessUser, decodeMemoryStat and decodeCpuStat unchanged.
274-289:⚠️ Potential issue | 🟡 Minor
@Suppress("MagicNumber")ondecodeCommand— same issue.Hardcoded field numbers (1, 8) should be extracted as constants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt` around lines 274 - 289, The decodeCommand function uses hardcoded protobuf field tags (1 and 8) — extract those into named constants and use them instead of magic numbers; e.g., define ARG_FIELD_NUMBER = 1 and EXE_FIELD_NUMBER = 8 (and/or TAG_ARG = (ARG_FIELD_NUMBER shl 3) or 2, TAG_EXE = (EXE_FIELD_NUMBER shl 3) or 2) in the file or companion object, then replace the literal cases ((1 shl 3) or 2 and (8 shl 3) or 2) in decodeCommand with the new constants (and remove or narrow the `@Suppress` if desired).backend/src/main/kotlin/com/moneat/logs/services/LogService.kt (1)
343-358:⚠️ Potential issue | 🟠 MajorRedact the success-path query log too.
Line 350 still writes
request.queryand the generated SQL condition verbatim. That undoes the new fingerprinted fallback/error logging because every successfully parsed search can still leak user-entered text and filters.🔐 Suggested fix
- logger.info { "Generated query condition from '${request.query}': $queryCondition" } + logger.debug { + "Generated query condition " + + "(query_fp=${utf8Fingerprint(request.query.orEmpty())}, " + + "condition_fp=${utf8Fingerprint(queryCondition)})" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt` around lines 343 - 358, The success-path info log currently prints request.query and the generated SQL verbatim (in the LogService block that calls queryParser.parse and logs "Generated query condition from '${request.query}': $queryCondition"), which leaks user input; change that logger.info call to avoid raw request.query by logging only the utf8Fingerprint(request.query) or a masked value instead and keep the generated queryCondition or its fingerprinted form—i.e., replace embedding of request.query with utf8Fingerprint(q) (or another redacted token) while preserving context and the $queryCondition in the message so debugging remains possible without exposing sensitive input.backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt (1)
172-172:⚠️ Potential issue | 🟠 Major
MagicNumberis still present in executable code.Lines 172 and 358 still default
retentionDaysto90, and Line 306 still hardcodes the DateTime64 precision as3. The PR objective is zero DetektMagicNumberfindings, so these should also be routed through named constants.🧹 Suggested cleanup
companion object { + private const val DEFAULT_RETENTION_DAYS = 90 private const val MAX_QUERY_RESULT_LIMIT = 10000 private const val EPOCH_MS_TO_SECONDS_DIVISOR = 1000.0 private const val DATETIME64_MILLIS_PRECISION = 3 @@ - retentionDays: Int = 90 + retentionDays: Int = DEFAULT_RETENTION_DAYS @@ - return "toDateTime64('$escaped', 3)" + return "toDateTime64('$escaped', $DATETIME64_MILLIS_PRECISION)" @@ - retentionDays: Int = 90 + retentionDays: Int = DEFAULT_RETENTION_DAYSAs per coding guidelines, "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))."Also applies to: 306-306, 358-358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt` at line 172, Introduce named constants in DashboardQueryEngine to replace the hardcoded numbers: add e.g. private const val DEFAULT_RETENTION_DAYS = 90 and private const val DATE_TIME64_PRECISION_MILLIS = 3 (names can vary but must be descriptive) and then replace the literal 90 used as the default for the retentionDays parameter (both occurrences of the retentionDays default) with DEFAULT_RETENTION_DAYS and replace the literal 3 used when constructing/formatting DateTime64 precision with DATE_TIME64_PRECISION_MILLIS; update any references in methods inside the DashboardQueryEngine class so Detekt flags for MagicNumber are removed.
♻️ Duplicate comments (1)
backend/src/main/kotlin/com/moneat/logs/services/LogService.kt (1)
622-624:⚠️ Potential issue | 🟠 MajorThese body previews still leak backend data.
Line 622 logs the raw aggregation payload on success, and Lines 624, 769, 923, 1025, and 1179 still log raw ClickHouse body text on failure. Those bodies can include echoed SQL, user filters, or result values, so this reintroduces the same sensitive-data logging issue the earlier redaction was fixing.
🔐 Suggested fix pattern
- logger.error { - "ClickHouse export error. query_fp=${utf8Fingerprint(sql)}\n" + - "Error: ${body.take(ERROR_BODY_PREVIEW_CHARS)}" - } + logger.error { + "ClickHouse export error " + + "(query_fp=${utf8Fingerprint(sql)}, body_fp=${utf8Fingerprint(body)}, body_len=${body.length})" + }Apply the same fingerprint/length-only pattern to the other
body.take(...)log sites in this file.Also applies to: 769-769, 921-924, 1025-1025, 1179-1179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt` around lines 622 - 624, The logs currently print raw ClickHouse response bodies via body.take(...) (seen in the logger.debug and logger.warn branches and other similar sites), which leaks sensitive data; replace these direct previews with a non-sensitive fingerprint and length-only preview pattern: compute a short hash/fingerprint of body and log something like "fingerprint=<fingerprint> length=<body.length>" instead of body.take(...). Update all usages that reference WARN_BODY_PREVIEW_CHARS, ERROR_BODY_PREVIEW_CHARS and checks like isClickHouseError() (the logger.debug/logger.warn/logger.error call sites that currently use body.take(...)) to use the fingerprint+length pattern to avoid echoing SQL, filters or result values.
🧹 Nitpick comments (12)
backend/src/main/kotlin/com/moneat/summary/services/SummaryService.kt (1)
87-88: Consider refactoring existing constants to use the new base units for consistency.The pre-existing
WEEK_SECONDSandTWO_WEEKS_SECONDSconstants still use inline arithmetic with magic numbers (7 * 24 * 3600L). For consistency with the new constants, these could be refactored:♻️ Optional refactor for consistency
-private const val WEEK_SECONDS = 7 * 24 * 3600L -private const val TWO_WEEKS_SECONDS = 14 * 24 * 3600L +private const val WEEK_SECONDS = DAYS_IN_WEEK * HOURS_PER_DAY * SECONDS_PER_HOUR +private const val TWO_WEEKS_SECONDS = 2 * WEEK_SECONDSNote: This is optional since these lines weren't part of the current changes and the existing code is functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/summary/services/SummaryService.kt` around lines 87 - 88, Refactor the WEEK_SECONDS and TWO_WEEKS_SECONDS constants to derive from the established base unit constants (e.g., DAY_SECONDS) instead of using inline magic-number arithmetic; replace "7 * 24 * 3600L" and "14 * 24 * 3600L" with expressions that multiply the appropriate week-length (7 and 14) by the existing DAY_SECONDS constant so WEEK_SECONDS and TWO_WEEKS_SECONDS consistently use the shared base unit.backend/src/main/kotlin/com/moneat/org/services/AdminService.kt (2)
597-602: Inconsistent constant usage for plan costs.
TEAM_CLOUD_COST_PER_ORGis used for team, but2.0is hardcoded for pro. Consider adding aPRO_CLOUD_COST_PER_ORGconstant for consistency.♻️ Proposed fix
Add to companion object:
private const val PRO_CLOUD_COST_PER_ORG = 2.0Then update the map:
val costPerPlan = mapOf( "free" to 0.0, - "pro" to 2.0, + "pro" to PRO_CLOUD_COST_PER_ORG, "team" to TEAM_CLOUD_COST_PER_ORG )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt` around lines 597 - 602, The cost map in AdminService (the val costPerPlan mapping "pro" to a hardcoded 2.0 while "team" uses TEAM_CLOUD_COST_PER_ORG) is inconsistent; add a PRO_CLOUD_COST_PER_ORG constant in the AdminService companion object and replace the hardcoded 2.0 in the costPerPlan map with PRO_CLOUD_COST_PER_ORG so both plan costs use named constants (refer to ADMIN service's costPerPlan and TEAM_CLOUD_COST_PER_ORG when making the change).
657-662: Alert messages should use the threshold constants to avoid staleness.The percentages in alert strings are hardcoded while the actual thresholds use constants. If
STORAGE_WARNING_THRESHOLD_PCTorSTORAGE_CRITICAL_THRESHOLD_PCTchange, these messages would become inaccurate.♻️ Proposed fix
- if (storageUsedPercent > STORAGE_WARNING_THRESHOLD_PCT) { - alerts.add("Storage > 70% (consider adding block storage)") - } - if (storageUsedPercent > STORAGE_CRITICAL_THRESHOLD_PCT) { - alerts.add("Storage > 80% (scaling trigger)") - } + if (storageUsedPercent > STORAGE_WARNING_THRESHOLD_PCT) { + alerts.add("Storage > ${STORAGE_WARNING_THRESHOLD_PCT.toInt()}% (consider adding block storage)") + } + if (storageUsedPercent > STORAGE_CRITICAL_THRESHOLD_PCT) { + alerts.add("Storage > ${STORAGE_CRITICAL_THRESHOLD_PCT.toInt()}% (scaling trigger)") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt` around lines 657 - 662, The alert strings hardcode "70%" and "80%" while the logic uses constants STORAGE_WARNING_THRESHOLD_PCT and STORAGE_CRITICAL_THRESHOLD_PCT; update the alerts in AdminService (where storageUsedPercent is compared and alerts is populated) to interpolate or format the actual constant values into the messages (e.g., "Storage > {STORAGE_WARNING_THRESHOLD_PCT}% (consider adding block storage)" and similarly for STORAGE_CRITICAL_THRESHOLD_PCT) so the text always reflects the defined thresholds.backend/src/main/kotlin/com/moneat/billing/services/StripeService.kt (1)
830-832: Optional: extract repeated bytes-per-meter-unit divisor into one local value.Line 830 and Line 832 repeat the same divisor expression; a single local value would reduce drift risk in future edits.
♻️ Suggested refactor
val pendingBytes = row[Subscriptions.pending_overage_bytes] - val drainableUnits = pendingBytes / (BYTES_PER_GB / GB_METER_UNITS_PER_GB) + val bytesPerMeterUnit = BYTES_PER_GB / GB_METER_UNITS_PER_GB + val drainableUnits = pendingBytes / bytesPerMeterUnit if (drainableUnits > 0) { - val remainingBytes = pendingBytes - drainableUnits * (BYTES_PER_GB / GB_METER_UNITS_PER_GB) + val remainingBytes = pendingBytes - drainableUnits * bytesPerMeterUnit Subscriptions.update({ Subscriptions.id eq subscriptionId }) { it[pending_meter_units] = row[Subscriptions.pending_meter_units] + drainableUnits it[pending_overage_bytes] = remainingBytes } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/StripeService.kt` around lines 830 - 832, The divisor BYTES_PER_GB / GB_METER_UNITS_PER_GB is repeated; introduce a local val (e.g., bytesPerMeterUnit) and use it in the calculations for drainableUnits and remainingBytes to avoid duplication and drift—replace both occurrences in the block where drainableUnits and remainingBytes are computed so drainableUnits = pendingBytes / bytesPerMeterUnit and remainingBytes = pendingBytes - drainableUnits * bytesPerMeterUnit (references: drainableUnits, pendingBytes, BYTES_PER_GB, GB_METER_UNITS_PER_GB).backend/src/main/kotlin/com/moneat/datadog/services/DatadogPprofFlamegraphService.kt (1)
37-37: Consider aligning JFR constant naming withDatadogJfrFlamegraphService.This file uses
JFR_MAGIC_LAST_INDEX = 3, whileDatadogJfrFlamegraphServiceusesJFR_MAGIC_LAST_BYTE_INDEX = 3. Both files also define their ownJFR_MAGICbyte array. Consider:
- Aligning the constant name (
JFR_MAGIC_LAST_INDEXvsJFR_MAGIC_LAST_BYTE_INDEX) for consistency.- Optionally extracting shared JFR magic constants to a common location to avoid drift.
Also applies to: 443-449, 467-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/datadog/services/DatadogPprofFlamegraphService.kt` at line 37, Rename the inconsistent constant JFR_MAGIC_LAST_INDEX in DatadogPprofFlamegraphService to match DatadogJfrFlamegraphService's name (use JFR_MAGIC_LAST_BYTE_INDEX) and consolidate the duplicate JFR_MAGIC byte-array definitions by extracting shared JFR constants (JFR_MAGIC and JFR_MAGIC_LAST_BYTE_INDEX) into a common util/object (e.g., a new JfrConstants or DatadogJfrUtils) that both DatadogPprofFlamegraphService and DatadogJfrFlamegraphService reference; update usages in methods that currently reference JFR_MAGIC and JFR_MAGIC_LAST_INDEX (and the other duplicated occurrences) to import and use the centralized constants.backend/src/main/kotlin/com/moneat/billing/services/PricingTierService.kt (2)
414-456: Keep validation messages bound to constants to avoid stale text.Bounds now use constants in logic, but messages still embed
90,60, and3650. Interpolate constants in messages so logic and error text stay in sync.Suggested patch
- require(request.retentionDays in 1..MAX_RETENTION_DAYS) { "Retention days must be between 1 and 90" } + require(request.retentionDays in 1..MAX_RETENTION_DAYS) { + "Retention days must be between 1 and $MAX_RETENTION_DAYS" + } - require(request.logRetentionDays in 1..MAX_RETENTION_DAYS) { "Log retention days must be between 1 and 90" } + require(request.logRetentionDays in 1..MAX_RETENTION_DAYS) { + "Log retention days must be between 1 and $MAX_RETENTION_DAYS" + } - require(request.replayRetentionDays in 1..MAX_RETENTION_DAYS) { - "Replay retention days must be between 1 and 90" - } + require(request.replayRetentionDays in 1..MAX_RETENTION_DAYS) { + "Replay retention days must be between 1 and $MAX_RETENTION_DAYS" + } - require(request.llmRetentionDays in 1..MAX_RETENTION_DAYS) { "LLM retention days must be between 1 and 90" } + require(request.llmRetentionDays in 1..MAX_RETENTION_DAYS) { + "LLM retention days must be between 1 and $MAX_RETENTION_DAYS" + } - require(request.trialDays in 0..MAX_TRIAL_DAYS) { "Trial days must be between 0 and 60" } + require(request.trialDays in 0..MAX_TRIAL_DAYS) { + "Trial days must be between 0 and $MAX_TRIAL_DAYS" + } require(request.analyticsRetentionDays in 1..MAX_ANALYTICS_RETENTION_DAYS) { - "Analytics retention days must be between 1 and 3650" + "Analytics retention days must be between 1 and $MAX_ANALYTICS_RETENTION_DAYS" }As per coding guidelines,
**/*.kt: "Use named constants instead of magic numbers (e.g.,const val MAX_RETRIES = 5instead ofif (count > 5))".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/PricingTierService.kt` around lines 414 - 456, The require() calls in PricingTierService.kt use constants for bounds but hardcode numbers in the error strings (e.g., "90", "60", "3650"); update each error message to interpolate the matching constant (e.g., MAX_RETENTION_DAYS, MAX_TRIAL_DAYS, MAX_ANALYTICS_RETENTION_DAYS) so the text stays in sync with the logic — adjust messages in the validation block that references request.retentionDays, request.logRetentionDays, request.replayRetentionDays, request.llmRetentionDays, request.trialDays, and request.analyticsRetentionDays to use the constant names instead of literal numbers.
59-59: Consider a single shared source forDEFAULT_ANALYTICS_RETENTION_DAYS.
PricingTierServiceandRetentionPolicyServiceeach define1095separately. Centralizing this into a shared constant/object would prevent future drift between tier creation defaults and retention enforcement defaults.Also applies to: 257-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/PricingTierService.kt` at line 59, PricingTierService and RetentionPolicyService both hard-code DEFAULT_ANALYTICS_RETENTION_DAYS = 1095; extract this value into a single shared constant (e.g., object AnalyticsDefaults { const val DEFAULT_ANALYTICS_RETENTION_DAYS = 1095 } or a Constants object) and replace the local definitions in PricingTierService and RetentionPolicyService with references to that shared constant (update usages in the PricingTierService file including the other mentions around the 257-259 region to use AnalyticsDefaults.DEFAULT_ANALYTICS_RETENTION_DAYS), removing the duplicate local constants so both services use the single source of truth.backend/src/main/kotlin/com/moneat/config/ClickHouseMigrations.kt (1)
44-44: Consider sharing ClickHouse error truncation constant across config classes.
ERROR_BODY_MAX_LENis now duplicated in bothClickHouseMigrationsandClickHouseClient. A small shared constant (e.g.,ClickHouseConstants) would prevent drift.Also applies to: 184-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/config/ClickHouseMigrations.kt` at line 44, ERROR_BODY_MAX_LEN is duplicated between ClickHouseMigrations and ClickHouseClient; extract a single shared constant (e.g., object ClickHouseConstants { const val ERROR_BODY_MAX_LEN = 500 }) and replace the local declarations with references to ClickHouseConstants.ERROR_BODY_MAX_LEN in both ClickHouseMigrations and ClickHouseClient; update imports/usages so both classes reference the new constant and remove the duplicated private const vals to prevent drift.backend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.kt (1)
62-65: ReuseMICROS_PER_CENTinpaygUsedCentsEstimatefor full consistency.There’s still one hardcoded
10_000LintoUsageResponse(Line 957). Reusing the same constant avoids split definitions of the same unit conversion.As per coding guidelines, "Use named constants instead of magic numbers (e.g., `const val MAX_RETRIES = 5` instead of `if (count > 5)`)."♻️ Suggested diff
- paygUsedCentsEstimate = (state.paygUsedMicros / 10_000L).toInt(), + paygUsedCentsEstimate = (state.paygUsedMicros / MICROS_PER_CENT).toInt(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.kt` around lines 62 - 65, Replace the hardcoded 10_000L used in the paygUsedCentsEstimate / toUsageResponse calculation with the existing MICROS_PER_CENT constant to ensure consistent unit conversion; update the arithmetic in the paygUsedCentsEstimate function (and the usage of the value in toUsageResponse) to reference MICROS_PER_CENT instead of the literal 10_000L so all micro-to-cent conversions use the single named constant.ee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.kt (1)
385-393: Optional: addHOURS_1Hfor complete symmetry inparseTimeRangeHours.You already constantized all other branches, so this is the last small literal in that mapping.
As per coding guidelines, "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))."Also applies to: 402-407
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.kt` around lines 385 - 393, In parseTimeRangeHours replace the literal "1" with a named constant HOURS_1H for symmetry with the other branches: update the when branch for "1h" to return HOURS_1H and add/ensure the HOURS_1H constant is defined alongside HOURS_6H, HOURS_24H, etc.; also apply the same change to the similar mapping used around the other occurrence referenced (the block at lines ~402-407) so no magic-number literals remain in either mapping.backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt (1)
249-254: Keep the cluster split limit tied to the last parsed index.Line 353 still hardcodes
limit = 9while the slot field now comes fromCLUSTER_NODE_SLOT_IDX = 8. If those constants change later, the split limit can silently fall out of sync and truncate the slot column. Derive the limit from the last index instead.♻️ Suggested cleanup
private const val CLUSTER_NODE_SLOT_IDX = 8 + private const val CLUSTER_NODE_PARTS_LIMIT = CLUSTER_NODE_SLOT_IDX + 1 @@ - val parts = line.split(" ", limit = 9) + val parts = line.split(" ", limit = CLUSTER_NODE_PARTS_LIMIT)As per coding guidelines, "Use named constants instead of magic numbers (e.g.,
const val MAX_RETRIES = 5instead ofif (count > 5))."Also applies to: 353-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt` around lines 249 - 254, The split limit for parsing cluster node fields is hardcoded to 9 and can get out of sync with the declared field indices; replace the magic number with a derived value based on the last declared index (use CLUSTER_NODE_SLOT_IDX + 1 or a named constant derived from CLUSTER_NODE_SLOT_IDX) wherever the split limit is set so the split range always follows the CLUSTER_NODE_* constants (e.g., update the code that currently uses limit = 9 to compute limit from CLUSTER_NODE_SLOT_IDX).backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt (1)
90-91: Extract the demotoDateTime64(...)builder to the shared ClickHouse utils.This now duplicates the same divisor/precision pair already used in
backend/src/main/kotlin/com/moneat/utils/ClickHouseQueryUtils.kt:76-90. If one copy changes later, demo-mode retention filtering and range filtering can drift apart. A tiny shared helper would keep both call sites aligned.Also applies to: 274-275
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt` around lines 90 - 91, Duplicate constants EPOCH_MS_TO_SECONDS_DIVISOR and DATETIME64_MILLIS_PRECISION and the demo toDateTime64(...) builder should be extracted into a single shared helper in the ClickHouse utils so both call sites use the same logic; add a helper (e.g. toDateTime64FromMillis(msExpr: String): String) in ClickHouseQueryUtils and replace the inline usage in DashboardQueryEngine (and the other occurrence at the indicated lines) to call that helper, removing the duplicated constants from DashboardQueryEngine and ensuring the helper uses EPOCH_MS_TO_SECONDS_DIVISOR and DATETIME64_MILLIS_PRECISION centrally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/kotlin/com/moneat/ai/AiChatService.kt`:
- Around line 31-32: Replace the current behavior that injects the first
LOG_CONTEXT_LENGTH chars of request.message into the unsupported-operation
exception with non-sensitive diagnostics: update the code in AiChatService where
the exception is constructed (the
unsupported-operation/unsupportedOperationException path) to include only safe
metadata such as request.message.length, a generated requestId or correlationId,
and/or a truncated hash (e.g. SHA-256) of the message instead of the raw
substring; reference LOG_CONTEXT_LENGTH and request.message in your change so
you remove the direct message echo while preserving useful debug info.
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt`:
- Around line 92-93: The code currently uses QUERY_LOG_PREVIEW_CHARS and
ERROR_BODY_PREVIEW_CHARS to emit truncated raw SQL and ClickHouse bodies (in
DashboardQueryEngine), which can leak sensitive filter/user/tenant values;
remove those raw previews and replace them with structured, non-sensitive
metadata: log a query id/correlation id, the sanitized query template or
parameterized fingerprint (no actual parameter values), execution duration, row
count, status/error code, and any hashed or redacted parameter keys if needed.
Specifically, stop using QUERY_LOG_PREVIEW_CHARS and ERROR_BODY_PREVIEW_CHARS in
the logging calls inside DashboardQueryEngine (where query/error previews are
emitted) and instead emit the structured fields above (e.g.,
template/fingerprint, durationMs, rowCount, errorType) so logs contain no raw
SQL or ClickHouse body content.
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/CloudWatchHandler.kt`:
- Around line 228-230: In CloudWatchHandler (the duration parsing branch that
handles "w", "M", "y"), guard the multiplication amount * DAYS_PER_WEEK /
DAYS_PER_MONTH / DAYS_PER_YEAR against Long overflow by performing a checked
multiply or bounding step before building the Duration string: compute a
safeDays value using a safeMultiplyOrCap(amount, DAYS_PER_*) helper (or validate
amount <= maxAllowedDays / DAYS_PER_*), reject or clamp values that would
overflow, and then pass safeDays into Duration.parse; ensure the validation
throws or returns a clear error for out-of-range input instead of producing an
invalid Duration.
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt`:
- Line 43: buildRedisUri currently strips schemes then unconditionally appends
:$port, which mangles inputs that already contain a port (e.g.,
rediss://cache.example.com:6380 -> ...:6380:6379); update buildRedisUri to
detect and preserve an existing port instead of appending the default one—either
by parsing the input with java.net.URI (or kotlin's URI) and rebuilding the
scheme/host/port properly, or by checking for a host portion that already
contains a ":" (including IPv6 bracket handling) and only appending :$port when
no port is present; update all call sites (calls from RedisHandler methods that
call buildRedisUri) to continue passing host/port but rely on the new normalized
URI logic.
In `@backend/src/main/kotlin/com/moneat/llm/services/LlmDashboardService.kt`:
- Line 48: The current code uses ERROR_BODY_PREVIEW_LENGTH and logs a slice of
the ClickHouse response body (around the logging call in
LlmDashboardService.kt), which can expose SQL and bound values; remove the
body-slice logging and instead log non-sensitive metadata such as response
status, context/endpoint, and the body length (e.g., response.contentLength or
body.length) where the current log references ERROR_BODY_PREVIEW_LENGTH; ensure
any remaining logs do not include raw body text or SQL, and update/remove the
ERROR_BODY_PREVIEW_LENGTH usage accordingly.
In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt`:
- Around line 1494-1496: The epoch-unit heuristic in the block that parses
`trimmed.toLongOrNull()` and uses `UNIX_SECONDS_THRESHOLD` incorrectly treats
valid millisecond timestamps (e.g., `946684800000`) as seconds; change the logic
in that return path (where `numeric` is handled) to distinguish units by digit
count or tighter cutoffs instead of the current `UNIX_SECONDS_THRESHOLD` check:
for example, check the length of `trimmed` (<=10 → seconds, 13 → milliseconds)
or use explicit numeric ranges (e.g., treat values >= 1_000_000_000_000 as
milliseconds and <= 9_999_999_999 as seconds) and only multiply by
`MS_PER_SECOND` when you’ve determined the value is in seconds; update the code
referencing `trimmed`, `numeric`, `UNIX_SECONDS_THRESHOLD`, and `MS_PER_SECOND`
accordingly.
In
`@ee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/IncidentRoutes.kt`:
- Line 64: The route currently parses the "limit" query param into 'limit' and
allows negative or huge values; update the handler in IncidentRoutes.kt to
validate and bound this value by rejecting out-of-range input with a
BadRequestException (or clamp it) instead of passing it downstream—use the
parsed 'limit', DEFAULT_INCIDENT_LIMIT, and a new MAX_INCIDENT_LIMIT constant
(or domain-appropriate upper bound) to enforce 0 < limit <= MAX_INCIDENT_LIMIT,
and throw BadRequestException with a clear message when the client-supplied
limit is invalid.
---
Outside diff comments:
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt`:
- Line 172: Introduce named constants in DashboardQueryEngine to replace the
hardcoded numbers: add e.g. private const val DEFAULT_RETENTION_DAYS = 90 and
private const val DATE_TIME64_PRECISION_MILLIS = 3 (names can vary but must be
descriptive) and then replace the literal 90 used as the default for the
retentionDays parameter (both occurrences of the retentionDays default) with
DEFAULT_RETENTION_DAYS and replace the literal 3 used when
constructing/formatting DateTime64 precision with DATE_TIME64_PRECISION_MILLIS;
update any references in methods inside the DashboardQueryEngine class so Detekt
flags for MagicNumber are removed.
In
`@backend/src/main/kotlin/com/moneat/datadog/decompression/ProcessAgentPayloadDecoder.kt`:
- Around line 321-335: Extract the hardcoded protobuf field numbers in
decodeCpuStat into named constants (e.g. CPU_TOTAL_PCT_FIELD = 2 and
CPU_NUM_THREADS_FIELD = 5) and replace occurrences of (2 shl 3) or 5 and (5 shl
3) or 0 with expressions that use those constants (e.g. (CPU_TOTAL_PCT_FIELD shl
3) or 5 or appropriate tag builder constants) so the magic numbers are
centralized; update the `@Suppress`("MagicNumber") usage accordingly and keep the
function signature decodeCpuStat(bytes: ByteArray): Pair<Float, Int> unchanged.
- Around line 136-165: The decodeContainer function currently uses magic numbers
for protobuf field tags and suppresses MagicNumber; instead, define named
constants for each field tag (e.g., PROTO_FIELD_ID/PROTO_FIELD_2,
PROTO_FIELD_NAME/PROTO_FIELD_3, PROTO_FIELD_IMAGE/PROTO_FIELD_4,
PROTO_FIELD_MEM_LIMIT/PROTO_FIELD_6, PROTO_FIELD_STATE/PROTO_FIELD_8,
PROTO_FIELD_NET_RCVD/PROTO_FIELD_16, PROTO_FIELD_NET_SENT/PROTO_FIELD_17,
PROTO_FIELD_TOTAL_PCT/PROTO_FIELD_20, PROTO_FIELD_MEM_RSS/PROTO_FIELD_21,
PROTO_FIELD_TAGS/PROTO_FIELD_26) matching the pattern used in
decodeCollectorContainer and other decoders, replace the numeric literals in the
when branches with those constants, and then remove the `@Suppress`("MagicNumber")
annotation.
- Around line 211-257: The decodeProcess method currently suppresses MagicNumber
and uses hardcoded protobuf tag numbers (e.g., (2 shl 3) or 0, (4 shl 3) or 2,
etc.); replace those literal tag expressions with descriptive constants (e.g.,
TAG_PID, TAG_COMMAND, TAG_USER, TAG_MEMORY, TAG_CPU, TAG_OPEN_FD, TAG_STATE,
TAG_TAGS) defined in a companion object or top-level area of
ProcessAgentPayloadDecoder, update the when branches to use those named
constants, remove the `@Suppress`("MagicNumber") annotation, and keep the existing
logic calling decodeCommand, decodeProcessUser, decodeMemoryStat and
decodeCpuStat unchanged.
- Around line 274-289: The decodeCommand function uses hardcoded protobuf field
tags (1 and 8) — extract those into named constants and use them instead of
magic numbers; e.g., define ARG_FIELD_NUMBER = 1 and EXE_FIELD_NUMBER = 8
(and/or TAG_ARG = (ARG_FIELD_NUMBER shl 3) or 2, TAG_EXE = (EXE_FIELD_NUMBER shl
3) or 2) in the file or companion object, then replace the literal cases ((1 shl
3) or 2 and (8 shl 3) or 2) in decodeCommand with the new constants (and remove
or narrow the `@Suppress` if desired).
In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt`:
- Around line 343-358: The success-path info log currently prints request.query
and the generated SQL verbatim (in the LogService block that calls
queryParser.parse and logs "Generated query condition from '${request.query}':
$queryCondition"), which leaks user input; change that logger.info call to avoid
raw request.query by logging only the utf8Fingerprint(request.query) or a masked
value instead and keep the generated queryCondition or its fingerprinted
form—i.e., replace embedding of request.query with utf8Fingerprint(q) (or
another redacted token) while preserving context and the $queryCondition in the
message so debugging remains possible without exposing sensitive input.
---
Duplicate comments:
In `@backend/src/main/kotlin/com/moneat/logs/services/LogService.kt`:
- Around line 622-624: The logs currently print raw ClickHouse response bodies
via body.take(...) (seen in the logger.debug and logger.warn branches and other
similar sites), which leaks sensitive data; replace these direct previews with a
non-sensitive fingerprint and length-only preview pattern: compute a short
hash/fingerprint of body and log something like "fingerprint=<fingerprint>
length=<body.length>" instead of body.take(...). Update all usages that
reference WARN_BODY_PREVIEW_CHARS, ERROR_BODY_PREVIEW_CHARS and checks like
isClickHouseError() (the logger.debug/logger.warn/logger.error call sites that
currently use body.take(...)) to use the fingerprint+length pattern to avoid
echoing SQL, filters or result values.
---
Nitpick comments:
In `@backend/src/main/kotlin/com/moneat/billing/services/BillingQuotaService.kt`:
- Around line 62-65: Replace the hardcoded 10_000L used in the
paygUsedCentsEstimate / toUsageResponse calculation with the existing
MICROS_PER_CENT constant to ensure consistent unit conversion; update the
arithmetic in the paygUsedCentsEstimate function (and the usage of the value in
toUsageResponse) to reference MICROS_PER_CENT instead of the literal 10_000L so
all micro-to-cent conversions use the single named constant.
In `@backend/src/main/kotlin/com/moneat/billing/services/PricingTierService.kt`:
- Around line 414-456: The require() calls in PricingTierService.kt use
constants for bounds but hardcode numbers in the error strings (e.g., "90",
"60", "3650"); update each error message to interpolate the matching constant
(e.g., MAX_RETENTION_DAYS, MAX_TRIAL_DAYS, MAX_ANALYTICS_RETENTION_DAYS) so the
text stays in sync with the logic — adjust messages in the validation block that
references request.retentionDays, request.logRetentionDays,
request.replayRetentionDays, request.llmRetentionDays, request.trialDays, and
request.analyticsRetentionDays to use the constant names instead of literal
numbers.
- Line 59: PricingTierService and RetentionPolicyService both hard-code
DEFAULT_ANALYTICS_RETENTION_DAYS = 1095; extract this value into a single shared
constant (e.g., object AnalyticsDefaults { const val
DEFAULT_ANALYTICS_RETENTION_DAYS = 1095 } or a Constants object) and replace the
local definitions in PricingTierService and RetentionPolicyService with
references to that shared constant (update usages in the PricingTierService file
including the other mentions around the 257-259 region to use
AnalyticsDefaults.DEFAULT_ANALYTICS_RETENTION_DAYS), removing the duplicate
local constants so both services use the single source of truth.
In `@backend/src/main/kotlin/com/moneat/billing/services/StripeService.kt`:
- Around line 830-832: The divisor BYTES_PER_GB / GB_METER_UNITS_PER_GB is
repeated; introduce a local val (e.g., bytesPerMeterUnit) and use it in the
calculations for drainableUnits and remainingBytes to avoid duplication and
drift—replace both occurrences in the block where drainableUnits and
remainingBytes are computed so drainableUnits = pendingBytes / bytesPerMeterUnit
and remainingBytes = pendingBytes - drainableUnits * bytesPerMeterUnit
(references: drainableUnits, pendingBytes, BYTES_PER_GB, GB_METER_UNITS_PER_GB).
In `@backend/src/main/kotlin/com/moneat/config/ClickHouseMigrations.kt`:
- Line 44: ERROR_BODY_MAX_LEN is duplicated between ClickHouseMigrations and
ClickHouseClient; extract a single shared constant (e.g., object
ClickHouseConstants { const val ERROR_BODY_MAX_LEN = 500 }) and replace the
local declarations with references to ClickHouseConstants.ERROR_BODY_MAX_LEN in
both ClickHouseMigrations and ClickHouseClient; update imports/usages so both
classes reference the new constant and remove the duplicated private const vals
to prevent drift.
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt`:
- Around line 90-91: Duplicate constants EPOCH_MS_TO_SECONDS_DIVISOR and
DATETIME64_MILLIS_PRECISION and the demo toDateTime64(...) builder should be
extracted into a single shared helper in the ClickHouse utils so both call sites
use the same logic; add a helper (e.g. toDateTime64FromMillis(msExpr: String):
String) in ClickHouseQueryUtils and replace the inline usage in
DashboardQueryEngine (and the other occurrence at the indicated lines) to call
that helper, removing the duplicated constants from DashboardQueryEngine and
ensuring the helper uses EPOCH_MS_TO_SECONDS_DIVISOR and
DATETIME64_MILLIS_PRECISION centrally.
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt`:
- Around line 249-254: The split limit for parsing cluster node fields is
hardcoded to 9 and can get out of sync with the declared field indices; replace
the magic number with a derived value based on the last declared index (use
CLUSTER_NODE_SLOT_IDX + 1 or a named constant derived from
CLUSTER_NODE_SLOT_IDX) wherever the split limit is set so the split range always
follows the CLUSTER_NODE_* constants (e.g., update the code that currently uses
limit = 9 to compute limit from CLUSTER_NODE_SLOT_IDX).
In
`@backend/src/main/kotlin/com/moneat/datadog/services/DatadogPprofFlamegraphService.kt`:
- Line 37: Rename the inconsistent constant JFR_MAGIC_LAST_INDEX in
DatadogPprofFlamegraphService to match DatadogJfrFlamegraphService's name (use
JFR_MAGIC_LAST_BYTE_INDEX) and consolidate the duplicate JFR_MAGIC byte-array
definitions by extracting shared JFR constants (JFR_MAGIC and
JFR_MAGIC_LAST_BYTE_INDEX) into a common util/object (e.g., a new JfrConstants
or DatadogJfrUtils) that both DatadogPprofFlamegraphService and
DatadogJfrFlamegraphService reference; update usages in methods that currently
reference JFR_MAGIC and JFR_MAGIC_LAST_INDEX (and the other duplicated
occurrences) to import and use the centralized constants.
In `@backend/src/main/kotlin/com/moneat/org/services/AdminService.kt`:
- Around line 597-602: The cost map in AdminService (the val costPerPlan mapping
"pro" to a hardcoded 2.0 while "team" uses TEAM_CLOUD_COST_PER_ORG) is
inconsistent; add a PRO_CLOUD_COST_PER_ORG constant in the AdminService
companion object and replace the hardcoded 2.0 in the costPerPlan map with
PRO_CLOUD_COST_PER_ORG so both plan costs use named constants (refer to ADMIN
service's costPerPlan and TEAM_CLOUD_COST_PER_ORG when making the change).
- Around line 657-662: The alert strings hardcode "70%" and "80%" while the
logic uses constants STORAGE_WARNING_THRESHOLD_PCT and
STORAGE_CRITICAL_THRESHOLD_PCT; update the alerts in AdminService (where
storageUsedPercent is compared and alerts is populated) to interpolate or format
the actual constant values into the messages (e.g., "Storage >
{STORAGE_WARNING_THRESHOLD_PCT}% (consider adding block storage)" and similarly
for STORAGE_CRITICAL_THRESHOLD_PCT) so the text always reflects the defined
thresholds.
In `@backend/src/main/kotlin/com/moneat/summary/services/SummaryService.kt`:
- Around line 87-88: Refactor the WEEK_SECONDS and TWO_WEEKS_SECONDS constants
to derive from the established base unit constants (e.g., DAY_SECONDS) instead
of using inline magic-number arithmetic; replace "7 * 24 * 3600L" and "14 * 24 *
3600L" with expressions that multiply the appropriate week-length (7 and 14) by
the existing DAY_SECONDS constant so WEEK_SECONDS and TWO_WEEKS_SECONDS
consistently use the shared base unit.
In
`@ee/backend/src/main/kotlin/com/moneat/enterprise/ai/services/EnterpriseAiChatService.kt`:
- Around line 385-393: In parseTimeRangeHours replace the literal "1" with a
named constant HOURS_1H for symmetry with the other branches: update the when
branch for "1h" to return HOURS_1H and add/ensure the HOURS_1H constant is
defined alongside HOURS_6H, HOURS_24H, etc.; also apply the same change to the
similar mapping used around the other occurrence referenced (the block at lines
~402-407) so no magic-number literals remain in either mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/dashboards/services/handlers/CloudWatchHandler.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt
Show resolved
Hide resolved
backend/src/main/kotlin/com/moneat/llm/services/LlmDashboardService.kt
Outdated
Show resolved
Hide resolved
ee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/IncidentRoutes.kt
Outdated
Show resolved
Hide resolved
- Exclude DemoDataReseeder.kt from MagicNumber rule instead of using @file:Suppress - Fix AdminService to use DAYS_IN_WEEK/DAYS_IN_MONTH instead of PERIOD_7D_DAYS_BACK (=6) for accurate 7-day and 30-day period calculations - Rename IngestRoutes ENVELOPE_LOG_MAX_LENGTH (500) to SHA256_HEX_PREFIX_CHARS (16) — SHA-256 hex is always 64 chars so 500 was a no-op; now logs a meaningful 16-char prefix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt`:
- Around line 216-224: In buildRedisUri, handle bare IPv6 addresses before
feeding the string to java.net.URI: detect when host contains ':' and is not
already bracketed (no leading '[' and trailing ']') and either wrap it with
brackets (e.g. "[host]") before prepending the scheme and parsing or explicitly
throw a clear validation error from the Ktor handler; update the logic around
normalized/parsed/cleanHost to use the bracketed form so java.net.URI can parse
correctly and avoid the substringBefore(':') fallback that produces "2001", and
add unit tests for IPv6 inputs (bare literal and already-bracketed) to cover
both success and rejection paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f6ae24a-87fd-40b1-8025-2ccdf3427a13
📒 Files selected for processing (10)
backend/src/main/kotlin/com/moneat/ai/AiChatService.ktbackend/src/main/kotlin/com/moneat/config/DemoDataReseeder.ktbackend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/CloudWatchHandler.ktbackend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.ktbackend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.ktbackend/src/main/kotlin/com/moneat/llm/services/LlmDashboardService.ktbackend/src/main/kotlin/com/moneat/logs/services/LogService.ktbackend/src/main/kotlin/com/moneat/org/services/AdminService.ktee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/IncidentRoutes.kt
✅ Files skipped from review due to trivial changes (5)
- backend/src/main/kotlin/com/moneat/events/routes/IngestRoutes.kt
- backend/src/main/kotlin/com/moneat/llm/services/LlmDashboardService.kt
- backend/src/main/kotlin/com/moneat/dashboards/services/handlers/CloudWatchHandler.kt
- backend/src/main/kotlin/com/moneat/config/DemoDataReseeder.kt
- backend/src/main/kotlin/com/moneat/logs/services/LogService.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- ee/backend/src/main/kotlin/com/moneat/enterprise/oncall/routes/IncidentRoutes.kt
- backend/src/main/kotlin/com/moneat/ai/AiChatService.kt
- backend/src/main/kotlin/com/moneat/dashboards/services/DashboardQueryEngine.kt
- backend/src/main/kotlin/com/moneat/org/services/AdminService.kt
backend/src/main/kotlin/com/moneat/dashboards/services/handlers/RedisHandler.kt
Outdated
Show resolved
Hide resolved
Adds unit tests covering all branches of recoverOnExpectedFailures, including CancellationException propagation and each expected failure type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Clarify that suppressions are acceptable for documented false positives and for using config-level exclusions when a rule genuinely doesn't fit a file context (e.g. seed data seeders for MagicNumber). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
❌ Quality Gate failed Issues Measures |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #279
Automated by auto-agent.
Summary by CodeRabbit