Skip to content

feat: implement comprehensive analytics and reporting system#239

Open
Whiznificent wants to merge 1 commit into
anonfedora:masterfrom
Whiznificent:feature/analytics-reporting-system
Open

feat: implement comprehensive analytics and reporting system#239
Whiznificent wants to merge 1 commit into
anonfedora:masterfrom
Whiznificent:feature/analytics-reporting-system

Conversation

@Whiznificent
Copy link
Copy Markdown

@Whiznificent Whiznificent commented Apr 25, 2026

  • Add dashboard metrics endpoint with real-time data
  • Implement transaction volume analysis with period-based reports
  • Add user analytics with behavior tracking and retention metrics
  • Create platform performance monitoring system
  • Implement compliance metrics for KYC/AML reporting
  • Add custom report generation with flexible filtering
  • Create time-series data aggregation service
  • Add data retention policies and export capabilities
  • Implement multi-level caching for performance optimization
  • Support JSON/CSV export formats for all reports

Summary

Closes #

Type of Change

  • Bug fix
  • New feature
  • Refactor / code cleanup
  • Docs / config update
  • Contract change (Soroban)
  • Breaking change

Changes Made

Testing

  • cargo test passes (contracts)
  • npm test passes (server)
  • tsc --noEmit passes (server)
  • cargo fmt + cargo clippy clean (contracts)
  • Manually tested locally

Contract Changes (if applicable)

  • ABI / interface changed
  • Migration required
  • Snapshot tests updated

Checklist

  • Self-reviewed the diff
  • No secrets or private keys committed
  • Relevant docs updated (README, .env.example, etc.)
  • PR title follows type(scope): description format (e.g. feat(escrow): add release timeout)

Closes #190

Summary by CodeRabbit

  • New Features
    • Added comprehensive analytics dashboard with metrics for platform performance and user behavior
    • Introduced volume and custom report generation with flexible parameters
    • Added compliance reporting capabilities (KYC, AML, audit, retention types)
    • Enabled data export functionality for compliance purposes
    • Implemented time-series data aggregation for trend analysis across configurable periods
    • Added automated data retention policies and cleanup management

- Add dashboard metrics endpoint with real-time data
- Implement transaction volume analysis with period-based reports
- Add user analytics with behavior tracking and retention metrics
- Create platform performance monitoring system
- Implement compliance metrics for KYC/AML reporting
- Add custom report generation with flexible filtering
- Create time-series data aggregation service
- Add data retention policies and export capabilities
- Implement multi-level caching for performance optimization
- Support JSON/CSV export formats for all reports

Resolves anonfedora#190
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

📝 Walkthrough

Walkthrough

The PR introduces a comprehensive analytics and reporting system by adding six new controller endpoints, corresponding service methods for dashboard metrics, volume reports, user behavior analysis, platform performance tracking, and compliance reporting, plus two new services for data retention and time-series aggregation with caching and database-backed computations.

Changes

Cohort / File(s) Summary
Analytics Controller
server/src/controllers/analytics.controller.ts
Added six new Express handlers (getDashboardMetrics, getVolumeReport, getUserAnalytics, getPlatformPerformance, generateCustomReport, getComplianceMetrics) that invoke service methods, cast query parameters (period, type) to string unions, and return results via res.json().
Analytics Routes
server/src/routes/analytics.routes.ts
Registered six new v1 analytics endpoints (GET /dashboard, /volume, /users, /performance, /compliance; POST /reports) wired to new controller exports, preserving existing legacy routes.
Analytics Service
server/src/services/analytics.service.ts
Added six public async methods (calculateDashboardMetrics, generateVolumeReport, analyzeUserBehavior, trackPlatformPerformance, generateComplianceReport, generateCustomReport) with typed return values, internal caching layers, Prisma database queries, and helper functions for metrics computation and report routing.
Data Retention Service
server/src/services/data-retention.service.ts
Introduced new singleton service with retention policies for audit logs, sessions, and analytics cache; provides cleanupExpiredData() for deletion, exportDataForCompliance() for JSON/CSV export, and status/policy management methods.
Time Series Service
server/src/services/time-series.service.ts
Introduced new singleton service with generateTimeSeriesReport() routing to specific aggregation methods (volume, user activity, transaction counts, performance, compliance) using raw SQL with dynamic period grouping, plus interval generation and export utilities.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as Analytics Controller
    participant Service as Analytics Service
    participant Cache
    participant Database
    
    Client->>Controller: POST /api/v1/analytics/reports<br/>(CustomReportRequest)
    Controller->>Service: generateCustomReport(request)
    alt Cache Hit
        Service->>Cache: checkCache(reportType)
        Cache-->>Service: cachedReport
    else Cache Miss
        Service->>Service: routeByReportType(reportType)
        alt Volume Report
            Service->>Database: query volume data
        else User Report
            Service->>Database: query user data
        else Compliance Report
            Service->>Database: query compliance data
        end
        Database-->>Service: raw data
        Service->>Service: computeMetadata(data)
        Service->>Cache: setCache(reportType, result)
    end
    Service-->>Controller: CustomReport
    Controller-->>Client: res.json(CustomReport)
Loading
sequenceDiagram
    participant Client
    participant Controller as Analytics Controller
    participant Service as Analytics Service
    participant TimeSeriesService
    participant Database
    
    Client->>Controller: GET /api/v1/analytics/dashboard
    Controller->>Service: calculateDashboardMetrics()
    Service->>Database: queryAuditLogs(), queryEscrows(), queryLoans()
    Database-->>Service: data
    Service->>Service: computeHealthStatus(data)
    Service->>Service: aggregateMetrics(data)
    Service->>Controller: DashboardMetrics
    Controller-->>Client: res.json(DashboardMetrics)
    
    Client->>Controller: GET /api/v1/analytics/volume?period=day
    Controller->>TimeSeriesService: generateTimeSeriesReport(type: 'volume', period: 'day')
    TimeSeriesService->>Database: aggregateVolumeData() via raw SQL<br/>(DATE_TRUNC by period)
    Database-->>TimeSeriesService: grouped sums
    TimeSeriesService-->>Controller: TimeSeriesData
    Controller-->>Client: res.json(TimeSeriesData)
Loading
sequenceDiagram
    participant Service as Data Retention Service
    participant Database
    participant Logger
    
    Service->>Service: cleanupExpiredData()
    loop For each enabled policy
        Service->>Database: calculateCutoffDate(policy.retentionDays)
        alt Policy Type: audit_logs
            Service->>Database: deleteMany(WHERE createdAt < cutoff)
            Database-->>Service: deletedCount
        else Policy Type: user_sessions
            Service->>Database: deleteMany(WHERE expiresAt < cutoff)
            Database-->>Service: deletedCount
        else Placeholder Types
            Service->>Logger: log(cutoff timestamp)
            Service-->>Service: return 0
        end
        Service->>Logger: logPolicyResult(policyName, deletedCount)
    end
    Service-->>Service: return { deletedRecords, policiesApplied }
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • anonfedora

Poem

🐰 A burrow of metrics, now shining so bright,
Time-series whispers from dawn until night,
Compliance and volumes dance through the cache,
Custom reports bloom in a data-filled stash! 📊✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately and concisely summarizes the main change: implementing a comprehensive analytics and reporting system, which matches the core objectives of adding multiple analytics endpoints, services, and reporting capabilities.
Description check ✅ Passed The description provides a clear bullet-point summary of key features implemented (dashboard metrics, volume analysis, user analytics, compliance metrics, custom reports, caching, export formats) and includes the related issue closure, meeting template requirements despite incomplete checklist sections.
Linked Issues check ✅ Passed All core requirements from issue #190 are met: six required endpoints are implemented with corresponding controller functions and routes; all five required service methods exist in AnalyticsService; time-series aggregation, caching, and export capabilities are added across new services.
Out of Scope Changes check ✅ Passed All changes align with the stated objectives: new analytics controller endpoints, route definitions, analytics service methods, time-series service, and data retention service directly implement the requirements specified in issue #190 with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Warning

⚠️ This pull request might be slop. It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Apr 25, 2026

@Whiznificent Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 12

🧹 Nitpick comments (3)
server/src/services/time-series.service.ts (2)

17-29: getTimeFormat is dead code.

getTimeFormat(period) is computed in aggregateVolumeData (line 17) and defined at lines 189–198, but it’s never used in any query — DATE_TRUNC is the only thing driving the grouping. Drop both the call and the helper, or wire it into the SELECT (e.g. TO_CHAR(DATE_TRUNC(...), ...)) if you actually need that formatting.

Also applies to: 189-198

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/time-series.service.ts` around lines 17 - 29, The call to
getTimeFormat(period) in aggregateVolumeData is dead code because the SQL uses
DATE_TRUNC for grouping; either remove the call and the getTimeFormat method
(delete getTimeFormat and its invocation in aggregateVolumeData) or wire the
format into the SQL by replacing DATE_TRUNC(...)::text with
TO_CHAR(DATE_TRUNC(${groupingColumn}, "createdAt"), ${timeFormat}) and using the
timeFormat from getTimeFormat(period) (ensure prisma.$queryRaw bindings and the
escrowData typing still match the returned period string).

21-29: Consider casting ${groupingColumn} to text in DATE_TRUNC calls, or extract the unit literal using Prisma.sql.

PostgreSQL's DATE_TRUNC requires the first argument (unit) to be a literal or properly-typed parameter. While Prisma's $queryRaw parameterizes ${groupingColumn} as a bind parameter, PostgreSQL may fail to infer its type with some connection poolers, raising "could not determine data type of parameter $1". Although the values here are enum-constrained ('minute', 'hour', 'day', 'week', 'month'), an explicit cast prevents potential issues:

DATE_TRUNC(${groupingColumn}::text, "createdAt")

Alternatively, use Prisma.sql to pass the unit as a literal. This pattern appears across multiple methods in the service (lines 23, 33, 58, 86, 96, 141, 151), so the fix should be applied consistently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/time-series.service.ts` around lines 21 - 29, The
DATE_TRUNC calls are passing ${groupingColumn} as a bind parameter which can
cause "could not determine data type" errors; update the raw queries (e.g., the
escrowData query using DATE_TRUNC(${groupingColumn}, "createdAt")) to either
cast the parameter to text (DATE_TRUNC(${groupingColumn}::text, "createdAt"))
or, better, pass the unit as a literal using Prisma.sql so Postgres sees a
proper unit literal; apply this change consistently to all similar queries in
this service (all DATE_TRUNC usages across the methods that construct escrowData
and the other time-series queries) so the grouping unit is always provided as a
text/literal rather than an untyped bind parameter.
server/src/services/analytics.service.ts (1)

506-506: Replace deprecated substr with crypto.randomUUID() for report ID generation.

String.prototype.substr is deprecated per MDN; Math.random().toString(36).substr(2, 9) is also collision-prone for a report ID that may be persisted or returned to clients. Use crypto.randomUUID() instead (available in Node ≥ 14.17, and your project requires Node ≥ 18).

Suggested change
-import { prisma } from "../services/database.service";
+import { randomUUID } from "crypto";
+import { prisma } from "../services/database.service";
@@
-        const reportId = `report_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
+        const reportId = `report_${randomUUID()}`;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/analytics.service.ts` at line 506, Replace the deprecated
Math.random(...).toString(36).substr(2, 9) with a cryptographically strong UUID:
change the reportId generation (the const reportId assignment) to use
crypto.randomUUID() so the ID becomes something like
"report_${Date.now()}_${crypto.randomUUID()}"; remove the substr usage and
ensure you call crypto.randomUUID() (available in Node ≥18) when constructing
reportId in analytics.service.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/src/controllers/analytics.controller.ts`:
- Around line 26-32: The handler getVolumeReport (and similarly the compliance
handler using generateComplianceReport) currently uses unsafe TypeScript casts
for req.query.period / req.query.type and req.query.userId; validate these query
values against an explicit allowed set (e.g. allowedPeriods =
['hour','day','week','month'] and allowedTypes = [...]) and if the incoming
value is not one of the allowed strings return res.status(400).json({ error:
'invalid period' }) or use a safe default, and ensure req.query.userId is a
string (not an array) before passing to the service; update the logic in
getVolumeReport and the compliance handler to perform these checks and only call
analyticsService.generateVolumeReport / generateComplianceReport with
validated/normalized values.
- Around line 49-55: The controller generateCustomReport currently forwards
req.body unchecked to analyticsService.generateCustomReport, causing bad input
to surface as 500s; add request validation at the controller boundary by
defining a small schema (e.g., using zod/joi/express-validator) that requires
reportType and ISO date strings for startDate/endDate and validates optional
format and filters, then parse/validate req.body inside generateCustomReport and
return a 400 with a clear error when validation fails before calling
analyticsService.generateCustomReport.

In `@server/src/routes/analytics.routes.ts`:
- Around line 6-16: The analytics routes expose sensitive data without
authentication and role checks; import and apply authMiddleware to the router
(e.g., add router.use(authMiddleware) at the top of analytics.routes.ts) so all
endpoints (getDashboardMetrics, getVolumeReport, getUserAnalytics,
getPlatformPerformance, generateCustomReport, getComplianceMetrics) require
authentication, and additionally protect the /reports and /compliance routes by
chaining requireRole(UserRole.ADMIN) on those specific route handlers to
restrict them to admins only.

In `@server/src/services/analytics.service.ts`:
- Around line 505-544: generateCustomReport is ignoring request.startDate,
request.endDate, request.filters and request.format: validate and normalize the
incoming dates from CustomReportRequest, then thread startDate/endDate and
filters into the underlying calls (replace hardcoded generateVolumeReport('day')
with generateVolumeReport(request.startDate, request.endDate, request.filters)
and likewise call analyzeUserBehavior(...), trackPlatformPerformance(...),
generateComplianceReport(...)) so each report honors the requested period and
filters; after obtaining data, if request.format === 'csv' call the appropriate
exporter on data (e.g. dataRetentionService.exportCsv or
timeSeriesService.toCsv) and set report.data to the serialized result (and set
metadata.format), otherwise keep JSON; preserve error handling and the generated
reportId/metadata.recordCount.
- Around line 369-424: analyzeUserBehavior builds a whereClause from userId but
never uses it, so per-user requests return global aggregates and cache behavior
is wrong; fix by applying the whereClause to all relevant queries (e.g., replace
prisma.user.count() and the active/new counts with prisma.user.count({ where:
whereClause }) or merged filters) and thread userId into dependent helpers by
changing getTopUserActions() and getUserGrowthTrend() to accept an optional
userId (e.g., getTopUserActions(userId) / getUserGrowthTrend(userId)) and filter
their DB queries by user; also ensure you skip reading/writing this.userCache
when userId is present (no caching for per-user results) or remove the userId
parameter entirely if per-user analytics are not supported.
- Around line 453-503: generateComplianceReport currently ignores its type param
and stores a single complianceCache, causing mismatched results; change it to
cache and compute per type by turning complianceCache into a map keyed by the
report type (e.g., complianceCache: Record<'kyc'|'aml'|'audit'|'retention',
{ts:number,data:ComplianceMetrics}>), check the keyed entry against
complianceTtl at the top of generateComplianceReport(type), and when missing or
expired only run the queries/logic required for that specific type (e.g., count
verified wallets for 'kyc', run AML auditLog queries for 'aml', call
dataRetentionService.getDataRetentionStatus() for 'retention', etc.), then store
the built ComplianceMetrics into complianceCache[type] before returning.
- Around line 281-300: The Promise.all call is combining two unawaited Promises
with a + which coerces them into a string, causing pendingCount to be "[object
Promise][object Promise]"; fix by moving prisma.escrow.count({ where: { status:
"PENDING" } }) and prisma.loan.count({ where: { status: "PENDING" } }) into two
separate entries in the Promise.all array (instead of adding them), await the
Promise.all as before, then compute pendingCount = Number(escrowCount) +
Number(loanCount) (or parseInt) after the await and use that numeric value for
pendingTransactions and in the DashboardMetrics object so the cached metrics
store a proper number.

In `@server/src/services/data-retention.service.ts`:
- Around line 284-299: The amlMetrics.reviewRate currently divides
flaggedTransactions by totalWallets (in the complianceData object), which mixes
incompatible counts; update amlMetrics.reviewRate to use a correct denominator
(e.g., totalTransactions or totalReviews) that represents the number of
transactions/reviews in the same window, or remove the reviewRate field
entirely; locate the complianceData construction and replace the expression
reviewRate: totalWallets > 0 ? (flaggedTransactions / totalWallets) * 100 : 0
with a calculation that uses a matching denominator (e.g., reviewRate:
totalTransactions > 0 ? (flaggedTransactions / totalTransactions) * 100 : 0) or
delete the reviewRate property.
- Around line 318-325: Change updateRetentionPolicy to only accept the existing
data-type union (use the same DataType type used by this.policies entries)
instead of string, and make retentionDays and enabled optional so callers can
perform partial updates; inside updateRetentionPolicy locate the policy via
this.policies.findIndex(p => p.dataType === dataType), throw if not found, and
only overwrite fields that were provided (leave other fields intact) to avoid
widening the union or flipping flags—this keeps updateRetentionPolicy consistent
with applyRetentionPolicy's expected data types and prevents default-case throws
during cleanup.
- Around line 128-138: The CSV exports (exportAuditLogs, exportUserData,
exportTransactionData, exportComplianceData) currently build rows with
row.join(',') and do not escape commas, double-quotes, newlines, or protect
against formula-injection (cells beginning with =,+,-,@); implement an escaping
helper (e.g., escapeCsvField(value): string) that applies RFC 4180 quoting
(double any internal quotes and wrap fields containing commas/newlines/quotes in
double-quotes) and prepends a single quote for values that start with =,+,-,or
@, then use this helper when mapping each field in the rows before joining with
commas (or replace whole implementation with a standard CSV library like
csv-stringify and ensure formula chars are prefixed). Ensure to call this helper
wherever rows are constructed in the four export functions.

In `@server/src/services/time-series.service.ts`:
- Around line 286-301: The CSV output in exportTimeSeriesData (async
exportTimeSeriesData) uses row.join(',') which will break on commas, quotes, or
newlines; replace this ad-hoc join with the shared CSV serializer utility used
in data-retention.service.ts (or add a common helper like serializeCsvRow /
escapeCsv) and import it into time-series.service.ts, then build rows as arrays
and call that serializer to produce each CSV line (ensuring values are quoted
when needed and inner quotes are doubled per RFC4180) and keep the headers
passed through the same serializer before joining with '\n'.
- Around line 131-164: getComplianceMetrics currently compares kycCompliance
(wallets with verifiedAt in the window) to totalWallets (wallets created in the
window), producing invalid rates; change the logic so both numerators and
denominators use the same cohort per period: either (A) per-period cohort =
wallets created in that period and count how many of those had verifiedAt <=
period_end (use DATE_TRUNC(${groupingColumn},"createdAt") for grouping, COUNT(*)
for total, and COUNT(*) FILTER (WHERE "verifiedAt" IS NOT NULL AND "verifiedAt"
<= period_end) for verified), or (B) compute running totals per period where
total = wallets created up to period_end and verified = wallets verified up to
period_end (use DATE_TRUNC(${groupingColumn}, timestamp) and <= period_end
conditions); update the SQL that populates kycCompliance and totalWallets
accordingly and ensure the arrays passed to calculateComplianceRates match the
same period keys and semantics (use the same DATE_TRUNC expression and period
boundaries for both queries).

---

Nitpick comments:
In `@server/src/services/analytics.service.ts`:
- Line 506: Replace the deprecated Math.random(...).toString(36).substr(2, 9)
with a cryptographically strong UUID: change the reportId generation (the const
reportId assignment) to use crypto.randomUUID() so the ID becomes something like
"report_${Date.now()}_${crypto.randomUUID()}"; remove the substr usage and
ensure you call crypto.randomUUID() (available in Node ≥18) when constructing
reportId in analytics.service.ts.

In `@server/src/services/time-series.service.ts`:
- Around line 17-29: The call to getTimeFormat(period) in aggregateVolumeData is
dead code because the SQL uses DATE_TRUNC for grouping; either remove the call
and the getTimeFormat method (delete getTimeFormat and its invocation in
aggregateVolumeData) or wire the format into the SQL by replacing
DATE_TRUNC(...)::text with TO_CHAR(DATE_TRUNC(${groupingColumn}, "createdAt"),
${timeFormat}) and using the timeFormat from getTimeFormat(period) (ensure
prisma.$queryRaw bindings and the escrowData typing still match the returned
period string).
- Around line 21-29: The DATE_TRUNC calls are passing ${groupingColumn} as a
bind parameter which can cause "could not determine data type" errors; update
the raw queries (e.g., the escrowData query using DATE_TRUNC(${groupingColumn},
"createdAt")) to either cast the parameter to text
(DATE_TRUNC(${groupingColumn}::text, "createdAt")) or, better, pass the unit as
a literal using Prisma.sql so Postgres sees a proper unit literal; apply this
change consistently to all similar queries in this service (all DATE_TRUNC
usages across the methods that construct escrowData and the other time-series
queries) so the grouping unit is always provided as a text/literal rather than
an untyped bind parameter.
🪄 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: 434b8f03-7efd-4bbf-a31a-d3e41709a77c

📥 Commits

Reviewing files that changed from the base of the PR and between 80c39ef and 10687af.

📒 Files selected for processing (5)
  • server/src/controllers/analytics.controller.ts
  • server/src/routes/analytics.routes.ts
  • server/src/services/analytics.service.ts
  • server/src/services/data-retention.service.ts
  • server/src/services/time-series.service.ts

Comment on lines +26 to +32
export async function getVolumeReport(req: Request, res: Response, next: NextFunction) {
try {
const period = req.query.period as 'hour' | 'day' | 'week' | 'month' || 'day';
const report = await analyticsService.generateVolumeReport(period);
res.json(report);
} catch (err) { next(err); }
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

as cast doesn’t validate — invalid period/type reach the service.

req.query.period as 'hour' | 'day' | 'week' | 'month' || 'day' and the type equivalent only lie to the type system. A request like ?period=foo reaches generateVolumeReport, where periodMap[period] is undefined, and new Date(now - undefined) is Invalid Date, which then poisons the volume cache for that key. Same hazard for type=foogenerateComplianceReport. Validate against the allowed set and 400 on mismatch (or default), and do the same for req.query.userId so a non-string array value doesn’t slip through.

🛡️ Minimal guard
+const PERIODS = ['hour', 'day', 'week', 'month'] as const;
+type Period = typeof PERIODS[number];
+const COMPLIANCE_TYPES = ['kyc', 'aml', 'audit', 'retention'] as const;
+type ComplianceType = typeof COMPLIANCE_TYPES[number];
@@
 export async function getVolumeReport(req: Request, res: Response, next: NextFunction) {
     try {
-        const period = req.query.period as 'hour' | 'day' | 'week' | 'month' || 'day';
+        const raw = typeof req.query.period === 'string' ? req.query.period : 'day';
+        if (!PERIODS.includes(raw as Period)) {
+            return res.status(400).json({ error: `Invalid period. Allowed: ${PERIODS.join(', ')}` });
+        }
+        const period = raw as Period;
         const report = await analyticsService.generateVolumeReport(period);
         res.json(report);
     } catch (err) { next(err); }
 }
@@
 export async function getComplianceMetrics(req: Request, res: Response, next: NextFunction) {
     try {
-        const type = req.query.type as 'kyc' | 'aml' | 'audit' | 'retention' || 'kyc';
+        const raw = typeof req.query.type === 'string' ? req.query.type : 'kyc';
+        if (!COMPLIANCE_TYPES.includes(raw as ComplianceType)) {
+            return res.status(400).json({ error: `Invalid type. Allowed: ${COMPLIANCE_TYPES.join(', ')}` });
+        }
+        const type = raw as ComplianceType;
         const metrics = await analyticsService.generateComplianceReport(type);
         res.json(metrics);
     } catch (err) { next(err); }
 }

Also applies to: 57-63

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/controllers/analytics.controller.ts` around lines 26 - 32, The
handler getVolumeReport (and similarly the compliance handler using
generateComplianceReport) currently uses unsafe TypeScript casts for
req.query.period / req.query.type and req.query.userId; validate these query
values against an explicit allowed set (e.g. allowedPeriods =
['hour','day','week','month'] and allowedTypes = [...]) and if the incoming
value is not one of the allowed strings return res.status(400).json({ error:
'invalid period' }) or use a safe default, and ensure req.query.userId is a
string (not an array) before passing to the service; update the logic in
getVolumeReport and the compliance handler to perform these checks and only call
analyticsService.generateVolumeReport / generateComplianceReport with
validated/normalized values.

Comment on lines +49 to +55
export async function generateCustomReport(req: Request, res: Response, next: NextFunction) {
try {
const reportRequest = req.body;
const report = await analyticsService.generateCustomReport(reportRequest);
res.json(report);
} catch (err) { next(err); }
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

generateCustomReport accepts req.body unchecked.

reportRequest = req.body is forwarded straight to the service with no schema validation, so a missing reportType or non-ISO startDate/endDate will surface as a 500 from deep inside the service rather than a 400. A small schema (zod/joi/express-validator) at the controller boundary would also enforce format and filters once the service starts honoring them.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/controllers/analytics.controller.ts` around lines 49 - 55, The
controller generateCustomReport currently forwards req.body unchecked to
analyticsService.generateCustomReport, causing bad input to surface as 500s; add
request validation at the controller boundary by defining a small schema (e.g.,
using zod/joi/express-validator) that requires reportType and ISO date strings
for startDate/endDate and validates optional format and filters, then
parse/validate req.body inside generateCustomReport and return a 400 with a
clear error when validation fails before calling
analyticsService.generateCustomReport.

Comment on lines +6 to +16
// Legacy endpoints
router.get("/", analyticsController.getPlatformStats);
router.get("/protocol", analyticsController.getProtocolAnalytics);

// New v1 analytics endpoints
router.get("/dashboard", analyticsController.getDashboardMetrics);
router.get("/volume", analyticsController.getVolumeReport);
router.get("/users", analyticsController.getUserAnalytics);
router.get("/performance", analyticsController.getPlatformPerformance);
router.post("/reports", analyticsController.generateCustomReport);
router.get("/compliance", analyticsController.getComplianceMetrics);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate where analytics.routes is mounted and check for auth middleware in the chain.
rg -nP --type=ts -C3 'analytics\.routes|analyticsRoutes|analyticsRouter'
# List middleware names used near analytics router registration.
rg -nP --type=ts -C5 "router\.use\(['\"][^'\"]*analytics" 
# Check for common auth middleware names referenced in the routes layer.
rg -nP --type=ts '\b(authenticate|requireAuth|isAuthenticated|requireRole|authMiddleware)\b' -g '!**/test/**'

Repository: anonfedora/stellovault

Length of output: 3834


🏁 Script executed:

cat -n server/src/routes/analytics.routes.ts

Repository: anonfedora/stellovault

Length of output: 921


🏁 Script executed:

head -100 server/src/app.ts | cat -n

Repository: anonfedora/stellovault

Length of output: 4443


Add authentication and role-based access control to analytics endpoints.

The /dashboard, /volume, /users, /performance, /reports, and /compliance endpoints expose sensitive platform metrics, financial data, user behavior analytics, and compliance/KYC-AML data without any authentication or authorization middleware. The analytics.routes.ts file contains no authMiddleware import or router.use(authMiddleware) call, and app.ts does not apply authentication globally. Unlike other protected routes in the codebase (wallet.routes, escrow.routes, loan.routes), analytics endpoints are publicly accessible.

At minimum, add router.use(authMiddleware) at the top of analytics.routes.ts. Additionally, /compliance and /reports should be restricted to admin-only via requireRole(UserRole.ADMIN).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/routes/analytics.routes.ts` around lines 6 - 16, The analytics
routes expose sensitive data without authentication and role checks; import and
apply authMiddleware to the router (e.g., add router.use(authMiddleware) at the
top of analytics.routes.ts) so all endpoints (getDashboardMetrics,
getVolumeReport, getUserAnalytics, getPlatformPerformance, generateCustomReport,
getComplianceMetrics) require authentication, and additionally protect the
/reports and /compliance routes by chaining requireRole(UserRole.ADMIN) on those
specific route handlers to restrict them to admins only.

Comment on lines +281 to +300
prisma.$queryRaw<Array<{ sum_amount: any }>>`
SELECT COALESCE(SUM("amount"), 0) AS sum_amount
FROM "Escrow"
WHERE "createdAt" >= ${new Date(Date.now() - 24 * 60 * 60 * 1000)}
`,
prisma.escrow.count({ where: { status: "PENDING" } }) +
prisma.loan.count({ where: { status: "PENDING" } }),
this.getSystemMetrics()
]);

const tvl = (tvlRes as any)?._sum?.amount ?? 0;
const dailyVolume = dailyVolumeRes?.[0]?.sum_amount ?? 0;

const systemHealth = this.calculateSystemHealth(systemMetrics);

const data: DashboardMetrics = {
totalValueLocked: this.toNumber(tvl).toString(),
activeUsers,
dailyTransactionVolume: this.toNumber(dailyVolume).toString(),
pendingTransactions: pendingCount,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical bug: pendingCount is "[object Promise][object Promise]", not a number.

The 4th element of the Promise.all array is

prisma.escrow.count({ where: { status: "PENDING" } }) +
prisma.loan.count({ where: { status: "PENDING" } })

That + is applied to two unawaited Promise objects, which JS coerces via toString(), so the array entry is the string "[object Promise][object Promise]". Promise.all then resolves it as-is, pendingCount is that string, and pendingTransactions in the response is a non-numeric string typed as number. The cached DashboardMetrics is also poisoned for the next 30s. Split the two counts into their own array entries and sum after the await.

🐛 Proposed fix
             const [
                 tvlRes,
                 activeUsers,
                 dailyVolumeRes,
-                pendingCount,
+                pendingEscrows,
+                pendingLoans,
                 systemMetrics
             ] = await Promise.all([
                 prisma.escrow.aggregate({
                     _sum: { amount: true },
                     where: { status: { in: ["FUNDED", "DISPUTED"] } }
                 }),
                 prisma.user.count({
                     where: {
                         updatedAt: {
                             gte: new Date(Date.now() - 24 * 60 * 60 * 1000)
                         }
                     }
                 }),
                 prisma.$queryRaw<Array<{ sum_amount: any }>>`
                     SELECT COALESCE(SUM("amount"), 0) AS sum_amount
                     FROM "Escrow"
                     WHERE "createdAt" >= ${new Date(Date.now() - 24 * 60 * 60 * 1000)}
                 `,
-                prisma.escrow.count({ where: { status: "PENDING" } }) +
-                prisma.loan.count({ where: { status: "PENDING" } }),
+                prisma.escrow.count({ where: { status: "PENDING" } }),
+                prisma.loan.count({ where: { status: "PENDING" } }),
                 this.getSystemMetrics()
             ]);
+
+            const pendingCount = pendingEscrows + pendingLoans;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
prisma.$queryRaw<Array<{ sum_amount: any }>>`
SELECT COALESCE(SUM("amount"), 0) AS sum_amount
FROM "Escrow"
WHERE "createdAt" >= ${new Date(Date.now() - 24 * 60 * 60 * 1000)}
`,
prisma.escrow.count({ where: { status: "PENDING" } }) +
prisma.loan.count({ where: { status: "PENDING" } }),
this.getSystemMetrics()
]);
const tvl = (tvlRes as any)?._sum?.amount ?? 0;
const dailyVolume = dailyVolumeRes?.[0]?.sum_amount ?? 0;
const systemHealth = this.calculateSystemHealth(systemMetrics);
const data: DashboardMetrics = {
totalValueLocked: this.toNumber(tvl).toString(),
activeUsers,
dailyTransactionVolume: this.toNumber(dailyVolume).toString(),
pendingTransactions: pendingCount,
const [
tvlRes,
activeUsers,
dailyVolumeRes,
pendingEscrows,
pendingLoans,
systemMetrics
] = await Promise.all([
prisma.escrow.aggregate({
_sum: { amount: true },
where: { status: { in: ["FUNDED", "DISPUTED"] } }
}),
prisma.user.count({
where: {
updatedAt: {
gte: new Date(Date.now() - 24 * 60 * 60 * 1000)
}
}
}),
prisma.$queryRaw<Array<{ sum_amount: any }>>`
SELECT COALESCE(SUM("amount"), 0) AS sum_amount
FROM "Escrow"
WHERE "createdAt" >= ${new Date(Date.now() - 24 * 60 * 60 * 1000)}
`,
prisma.escrow.count({ where: { status: "PENDING" } }),
prisma.loan.count({ where: { status: "PENDING" } }),
this.getSystemMetrics()
]);
const pendingCount = pendingEscrows + pendingLoans;
const tvl = (tvlRes as any)?._sum?.amount ?? 0;
const dailyVolume = dailyVolumeRes?.[0]?.sum_amount ?? 0;
const systemHealth = this.calculateSystemHealth(systemMetrics);
const data: DashboardMetrics = {
totalValueLocked: this.toNumber(tvl).toString(),
activeUsers,
dailyTransactionVolume: this.toNumber(dailyVolume).toString(),
pendingTransactions: pendingCount,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/analytics.service.ts` around lines 281 - 300, The
Promise.all call is combining two unawaited Promises with a + which coerces them
into a string, causing pendingCount to be "[object Promise][object Promise]";
fix by moving prisma.escrow.count({ where: { status: "PENDING" } }) and
prisma.loan.count({ where: { status: "PENDING" } }) into two separate entries in
the Promise.all array (instead of adding them), await the Promise.all as before,
then compute pendingCount = Number(escrowCount) + Number(loanCount) (or
parseInt) after the await and use that numeric value for pendingTransactions and
in the DashboardMetrics object so the cached metrics store a proper number.

Comment on lines +369 to +424
async analyzeUserBehavior(userId?: string): Promise<UserAnalytics> {
const now = Date.now();
if (this.userCache && now - this.userCache.ts < this.userTtl && !userId) {
return this.userCache.data;
}

try {
const whereClause = userId ? { id: userId } : {};

const [
totalUsers,
activeUsers,
newUsers,
userActions,
userGrowth
] = await Promise.all([
prisma.user.count(),
prisma.user.count({
where: {
updatedAt: {
gte: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000)
}
}
}),
prisma.user.count({
where: {
createdAt: {
gte: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000)
}
}
}),
this.getTopUserActions(),
this.getUserGrowthTrend()
]);

const retentionRate = totalUsers > 0 ? (activeUsers / totalUsers) * 100 : 0;

const data: UserAnalytics = {
totalUsers,
activeUsers,
newUsers,
userRetentionRate: retentionRate,
averageSessionDuration: 1800, // 30 minutes placeholder
topUserActions: userActions,
userGrowth
};

if (!userId) {
this.userCache = { ts: now, data };
}

return data;
} catch (err) {
throw err;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

analyzeUserBehavior(userId) ignores userId.

whereClause = userId ? { id: userId } : {} is built but never passed into any of the queries on lines 385–402, so calling GET /api/v1/analytics/users?userId=... returns the same global aggregate as the unfiltered call. The cache guard at line 371 also leaks: with a userId you skip the cache read but still don't compute anything user-specific. Either implement the per-user path (e.g. session/audit-log queries scoped to that user, no cache) or drop the parameter from the public surface so callers don’t assume it works.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/analytics.service.ts` around lines 369 - 424,
analyzeUserBehavior builds a whereClause from userId but never uses it, so
per-user requests return global aggregates and cache behavior is wrong; fix by
applying the whereClause to all relevant queries (e.g., replace
prisma.user.count() and the active/new counts with prisma.user.count({ where:
whereClause }) or merged filters) and thread userId into dependent helpers by
changing getTopUserActions() and getUserGrowthTrend() to accept an optional
userId (e.g., getTopUserActions(userId) / getUserGrowthTrend(userId)) and filter
their DB queries by user; also ensure you skip reading/writing this.userCache
when userId is present (no caching for per-user results) or remove the userId
parameter entirely if per-user analytics are not supported.

Comment on lines +128 to +138
const headers = ['id', 'userId', 'action', 'resourceId', 'ipAddress', 'createdAt'];
const rows = logs.map(log => [
log.id,
log.userId || '',
log.action,
log.resourceId || '',
log.ipAddress || '',
log.createdAt.toISOString()
]);

return [headers, ...rows].map(row => row.join(',')).join('\n');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

CSV outputs do not escape values — corruption and CSV injection risk.

All four CSV exports (exportAuditLogs, exportUserData, exportTransactionData, exportComplianceData) just row.join(',') without quoting fields or escaping commas, double quotes, newlines, or formula-leading characters (=, +, -, @). User-controlled values reach these rows (e.g. user.name, auditLog.action, auditLog.resourceId, escrow.assetCode), so a single name like O'Reilly, Inc. or =cmd|... will either break parsing or execute when the CSV is opened in Excel. Apply RFC 4180 quoting plus a leading ' for formula-prefixed cells (or use a CSV library like csv-stringify).

🛡️ Suggested escaping helper applied at all four CSV emit sites
+    private csvEscape(value: unknown): string {
+        const s = value == null ? '' : String(value);
+        // Neutralize spreadsheet formula injection
+        const safe = /^[=+\-@\t\r]/.test(s) ? `'${s}` : s;
+        // Quote and double-up embedded quotes per RFC 4180
+        if (/[",\r\n]/.test(safe)) {
+            return `"${safe.replace(/"/g, '""')}"`;
+        }
+        return safe;
+    }
+
+    private toCsv(headers: string[], rows: unknown[][]): string {
+        return [headers, ...rows]
+            .map(row => row.map(v => this.csvEscape(v)).join(','))
+            .join('\n');
+    }

Then replace each emit, e.g.:

-        return [headers, ...rows].map(row => row.join(',')).join('\n');
+        return this.toCsv(headers, rows);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const headers = ['id', 'userId', 'action', 'resourceId', 'ipAddress', 'createdAt'];
const rows = logs.map(log => [
log.id,
log.userId || '',
log.action,
log.resourceId || '',
log.ipAddress || '',
log.createdAt.toISOString()
]);
return [headers, ...rows].map(row => row.join(',')).join('\n');
const headers = ['id', 'userId', 'action', 'resourceId', 'ipAddress', 'createdAt'];
const rows = logs.map(log => [
log.id,
log.userId || '',
log.action,
log.resourceId || '',
log.ipAddress || '',
log.createdAt.toISOString()
]);
return this.toCsv(headers, rows);
private csvEscape(value: unknown): string {
const s = value == null ? '' : String(value);
// Neutralize spreadsheet formula injection
const safe = /^[=+\-@\t\r]/.test(s) ? `'${s}` : s;
// Quote and double-up embedded quotes per RFC 4180
if (/[",\r\n]/.test(safe)) {
return `"${safe.replace(/"/g, '""')}"`;
}
return safe;
}
private toCsv(headers: string[], rows: unknown[][]): string {
return [headers, ...rows]
.map(row => row.map(v => this.csvEscape(v)).join(','))
.join('\n');
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/data-retention.service.ts` around lines 128 - 138, The
CSV exports (exportAuditLogs, exportUserData, exportTransactionData,
exportComplianceData) currently build rows with row.join(',') and do not escape
commas, double-quotes, newlines, or protect against formula-injection (cells
beginning with =,+,-,@); implement an escaping helper (e.g.,
escapeCsvField(value): string) that applies RFC 4180 quoting (double any
internal quotes and wrap fields containing commas/newlines/quotes in
double-quotes) and prepends a single quote for values that start with =,+,-,or
@, then use this helper when mapping each field in the rows before joining with
commas (or replace whole implementation with a standard CSV library like
csv-stringify and ensure formula chars are prefixed). Ensure to call this helper
wherever rows are constructed in the four export functions.

Comment on lines +284 to +299
const complianceData = {
period: `${startDate.toISOString()} to ${endDate.toISOString()}`,
kycMetrics: {
totalWallets,
verifiedWallets,
complianceRate: totalWallets > 0 ? (verifiedWallets / totalWallets) * 100 : 0
},
amlMetrics: {
flaggedTransactions,
reviewRate: totalWallets > 0 ? (flaggedTransactions / totalWallets) * 100 : 0
},
auditTrail: {
integrity: true,
lastVerified: new Date().toISOString()
}
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

amlMetrics.reviewRate mixes incompatible numerators and denominators.

flaggedTransactions is a count of audit log rows containing FLAGGED, while totalWallets is the count of wallets created in the window. Dividing them produces a number that is neither a rate nor meaningful, and it can easily exceed 100% (e.g. many flags, few new wallets). Either remove reviewRate or compute it against an actual transaction/review denominator.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/data-retention.service.ts` around lines 284 - 299, The
amlMetrics.reviewRate currently divides flaggedTransactions by totalWallets (in
the complianceData object), which mixes incompatible counts; update
amlMetrics.reviewRate to use a correct denominator (e.g., totalTransactions or
totalReviews) that represents the number of transactions/reviews in the same
window, or remove the reviewRate field entirely; locate the complianceData
construction and replace the expression reviewRate: totalWallets > 0 ?
(flaggedTransactions / totalWallets) * 100 : 0 with a calculation that uses a
matching denominator (e.g., reviewRate: totalTransactions > 0 ?
(flaggedTransactions / totalTransactions) * 100 : 0) or delete the reviewRate
property.

Comment on lines +318 to +325
async updateRetentionPolicy(dataType: string, retentionDays: number, enabled: boolean): Promise<void> {
const policyIndex = this.policies.findIndex(p => p.dataType === dataType);
if (policyIndex === -1) {
throw new Error(`Unknown data type: ${dataType}`);
}

this.policies[policyIndex] = { dataType, retentionDays, enabled };
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

updateRetentionPolicy silently widens the policy union.

The parameter is dataType: string, so callers can insert a policy whose dataType is not in 'audit_logs' | 'user_sessions' | 'temp_data' | 'analytics_cache'. applyRetentionPolicy's switch would then fall into default and throw at the next cleanup. Constrain the parameter to the existing union (and likely keep enabled/days optional so a partial update doesn't silently flip flags).

♻️ Proposed signature
-    async updateRetentionPolicy(dataType: string, retentionDays: number, enabled: boolean): Promise<void> {
-        const policyIndex = this.policies.findIndex(p => p.dataType === dataType);
-        if (policyIndex === -1) {
-            throw new Error(`Unknown data type: ${dataType}`);
-        }
-
-        this.policies[policyIndex] = { dataType, retentionDays, enabled };
-    }
+    async updateRetentionPolicy(
+        dataType: RetentionPolicy['dataType'],
+        retentionDays: number,
+        enabled: boolean,
+    ): Promise<void> {
+        const policyIndex = this.policies.findIndex(p => p.dataType === dataType);
+        if (policyIndex === -1) {
+            throw new Error(`Unknown data type: ${dataType}`);
+        }
+        this.policies[policyIndex] = { dataType, retentionDays, enabled };
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async updateRetentionPolicy(dataType: string, retentionDays: number, enabled: boolean): Promise<void> {
const policyIndex = this.policies.findIndex(p => p.dataType === dataType);
if (policyIndex === -1) {
throw new Error(`Unknown data type: ${dataType}`);
}
this.policies[policyIndex] = { dataType, retentionDays, enabled };
}
async updateRetentionPolicy(
dataType: RetentionPolicy['dataType'],
retentionDays: number,
enabled: boolean,
): Promise<void> {
const policyIndex = this.policies.findIndex(p => p.dataType === dataType);
if (policyIndex === -1) {
throw new Error(`Unknown data type: ${dataType}`);
}
this.policies[policyIndex] = { dataType, retentionDays, enabled };
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/data-retention.service.ts` around lines 318 - 325, Change
updateRetentionPolicy to only accept the existing data-type union (use the same
DataType type used by this.policies entries) instead of string, and make
retentionDays and enabled optional so callers can perform partial updates;
inside updateRetentionPolicy locate the policy via this.policies.findIndex(p =>
p.dataType === dataType), throw if not found, and only overwrite fields that
were provided (leave other fields intact) to avoid widening the union or
flipping flags—this keeps updateRetentionPolicy consistent with
applyRetentionPolicy's expected data types and prevents default-case throws
during cleanup.

Comment on lines +131 to +164
async getComplianceMetrics(
startDate: Date,
endDate: Date,
period: AggregationPeriod
): Promise<TimeSeriesData[]> {
const groupingColumn = this.getGroupingColumn(period);

try {
const kycCompliance = await prisma.$queryRaw<Array<{ period: string; rate: any }>>`
SELECT
DATE_TRUNC(${groupingColumn}, "verifiedAt")::text as period,
COUNT(*) as rate
FROM "Wallet"
WHERE "verifiedAt" >= ${startDate} AND "verifiedAt" <= ${endDate}
GROUP BY DATE_TRUNC(${groupingColumn}, "verifiedAt")
ORDER BY period ASC
`;

const totalWallets = await prisma.$queryRaw<Array<{ period: string; total: any }>>`
SELECT
DATE_TRUNC(${groupingColumn}, "createdAt")::text as period,
COUNT(*) as total
FROM "Wallet"
WHERE "createdAt" >= ${startDate} AND "createdAt" <= ${endDate}
GROUP BY DATE_TRUNC(${groupingColumn}, "createdAt")
ORDER BY period ASC
`;

return this.calculateComplianceRates(kycCompliance, totalWallets);
} catch (error) {
console.error('Error aggregating compliance metrics:', error);
throw error;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

getComplianceMetrics divides verified-in-window by created-in-window.

kycCompliance counts wallets whose verifiedAt falls in [startDate, endDate], while totalWallets counts wallets whose createdAt falls in the same window — these are largely disjoint sets, and a wallet verified later than it was created will inflate the rate above 100%. For a meaningful per-period KYC rate, both should be computed over the same cohort (e.g. wallets created in the period, with verification status as of the period end), or total should simply be the running wallet count at each timestamp.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/time-series.service.ts` around lines 131 - 164,
getComplianceMetrics currently compares kycCompliance (wallets with verifiedAt
in the window) to totalWallets (wallets created in the window), producing
invalid rates; change the logic so both numerators and denominators use the same
cohort per period: either (A) per-period cohort = wallets created in that period
and count how many of those had verifiedAt <= period_end (use
DATE_TRUNC(${groupingColumn},"createdAt") for grouping, COUNT(*) for total, and
COUNT(*) FILTER (WHERE "verifiedAt" IS NOT NULL AND "verifiedAt" <= period_end)
for verified), or (B) compute running totals per period where total = wallets
created up to period_end and verified = wallets verified up to period_end (use
DATE_TRUNC(${groupingColumn}, timestamp) and <= period_end conditions); update
the SQL that populates kycCompliance and totalWallets accordingly and ensure the
arrays passed to calculateComplianceRates match the same period keys and
semantics (use the same DATE_TRUNC expression and period boundaries for both
queries).

Comment on lines +286 to +301
async exportTimeSeriesData(
data: TimeSeriesData[],
format: 'json' | 'csv'
): Promise<string> {
if (format === 'json') {
return JSON.stringify(data, null, 2);
}

// CSV format
const headers = ['timestamp', 'value'];
const rows = data.map(d => [d.timestamp, d.value.toString()]);

return [headers, ...rows]
.map(row => row.join(','))
.join('\n');
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same CSV escaping issue as data-retention.service.ts.

row.join(',') here will also break for any timestamp/value that contains a comma, quote or newline. Reuse a shared CSV serializer (see the helper suggested in the data-retention review).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/services/time-series.service.ts` around lines 286 - 301, The CSV
output in exportTimeSeriesData (async exportTimeSeriesData) uses row.join(',')
which will break on commas, quotes, or newlines; replace this ad-hoc join with
the shared CSV serializer utility used in data-retention.service.ts (or add a
common helper like serializeCsvRow / escapeCsv) and import it into
time-series.service.ts, then build rows as arrays and call that serializer to
produce each CSV line (ensuring values are quoted when needed and inner quotes
are doubled per RFC4180) and keep the headers passed through the same serializer
before joining with '\n'.

@anonfedora
Copy link
Copy Markdown
Owner

Let your server ci pass, please

@anonfedora
Copy link
Copy Markdown
Owner

Are you still available for this? All my passing CI are no longer passing from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ANALYTICS] Implement Analytics and Reporting System

2 participants