feat: Per-dimension stats API#587
Merged
Merged
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:
📝 WalkthroughWalkthroughThis PR introduces a full "Statistics API v2" — a per-dimension analytics read API with new registries, DTOs, filter/query builders, mappers, AnalyticsV2Service and SeoV2Service, controllers, and access protection, mirrored across the cloud and community backends, plus tests, docs, and tooling. The web app is migrated to consume v2 via a new API client, React Query hooks, updated filter UI, and rewritten view/tab components, while legacy v1 user-flow endpoints and several deferred-loading proxies are removed. ChangesBackend Analytics V2 API
Frontend V2 Migration
Estimated code review effort: 5 (Critical) | ~180 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant V2Controller
participant AnalyticsV2Service
participant ClickHouse
Client->>V2Controller: GET v2 analytics endpoint
V2Controller->>AnalyticsV2Service: assertReadAccess and operation request
AnalyticsV2Service->>AnalyticsV2Service: resolveTimeframe and prepareFilters
AnalyticsV2Service->>ClickHouse: execute generated analytics query
ClickHouse-->>AnalyticsV2Service: query results
AnalyticsV2Service-->>V2Controller: V2Envelope data and metadata
V2Controller-->>Client: JSON response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused export flagged by CI — likely a missed DRY opportunity.
V2_MAX_ENTITY_LIMITis exported but never imported outside this file, per the knip pipeline failures.V2FunnelSessionsQueryDtoinproject.dto.ts(Line 65) hardcodes the same value (150) instead of reusing this constant, andV2ErrorSessionsQueryDtohere duplicates the limit/offset pattern with a different hardcoded max (50). Either import/reuseV2_MAX_ENTITY_LIMITwhere the same 150 cap applies, or drop the export if it's not meant to be shared.♻️ Reuse the constant in project.dto.ts
+import { V2_MAX_ENTITY_LIMIT } from './entities.dto' ... - `@Max`(150) + `@Max`(V2_MAX_ENTITY_LIMIT) limit?: numberAlso applies to: 99-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts` at line 18, V2_MAX_ENTITY_LIMIT is exported but not reused, causing an unused export and duplicated hardcoded limits. Update the DTOs that cap entity/session results to import and use V2_MAX_ENTITY_LIMIT where the 150 limit applies, especially V2FunnelSessionsQueryDto in project.dto.ts, and make V2ErrorSessionsQueryDto follow the same shared pattern if appropriate; otherwise remove the export from entities.dto.ts if it is not intended to be shared. Keep the limit/offset decorators aligned with the existing DTO classes so the shared constant is the single source of truth.Source: Pipeline failures
backend/apps/cloud/src/analytics/v2/dto/project.dto.ts (1)
18-36: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEnforce the documented
steps/funnelIdexclusivity.getPagesArray()currently givesfunnelIdprecedence and ignoresstepswhen both are sent, and empty input falls through to a generic parse error. Add a cross-field validator or explicit 400 so invalid combinations fail fast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/dto/project.dto.ts` around lines 18 - 36, The V2FunnelQueryDto currently documents that steps and funnelId are mutually exclusive, but the request is still accepted when both are present and getPagesArray() silently prefers funnelId; add explicit cross-field validation on V2FunnelQueryDto (or equivalent 400 handling in the funnel query flow) so invalid combinations fail fast, and make sure getPagesArray() no longer relies on fallback precedence for conflicting or empty input.backend/apps/cloud/src/analytics/v2/dto/v2-base.dto.ts (1)
161-169: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider validating the
sortparam shape at the DTO layer.
sortaccepts any string and is only parsed/validated downstream (parseSortParam). A lightweight@Matches(/^[\w.]+:(asc|desc)$/)here would reject malformed input earlier with a clearer 400 instead of relying entirely on service-layer parsing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/dto/v2-base.dto.ts` around lines 161 - 169, The sort field in the V2 base DTO is only checked as a string, so malformed values are deferred to downstream parsing. Add a DTO-level shape validation on the sort property in v2-base.dto.ts, near the sort field decorators, by updating the V2BaseDto sort declaration to include a pattern check that only allows field:direction with asc or desc. Keep the existing ApiProperty, IsOptional, and IsString decorators, and use the sort property and parseSortParam flow as the main reference points when locating the change.backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeep relative import couples analytics mapper to user module internals.
../../../user/entities/user.entityreaches across domain boundaries for a single constant; consider re-exportingDEFAULT_TIMEZONEfrom a shared/common constants module to decouple analytics from the user entity's file layout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts` at line 5, The import in timeseries.mapper is reaching into the user entity internals for DEFAULT_TIMEZONE, which tightly couples analytics to that module layout. Move DEFAULT_TIMEZONE to a shared/common constants location (or re-export it from a shared entrypoint) and update the timeseries.mapper import to use that shared symbol instead of the deep relative path. Keep the mapper logic unchanged and use the existing DEFAULT_TIMEZONE identifier as the migration target.backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts (1)
55-208: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftRepeated auth/logging boilerplate across every handler.
Every handler repeats the same
logger.log(...)+assertReadAccess(pid, uid, headers['x-password'])sequence. This pattern is duplicated across all v2 controllers in this cohort (sessions, profiles, project). Consider consolidating into a param decorator, custom guard, or interceptor that resolvespid/uid/passwordand performs the access check once, so future endpoints/controllers can't accidentally skip it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts` around lines 55 - 208, The controller methods in ErrorsV2Controller repeat the same logging and access-check boilerplate in every handler, so consolidate that cross-cutting logic into a shared NestJS mechanism such as a custom guard, interceptor, or param decorator that can resolve pid, uid, and x-password and call analyticsV2Service.assertReadAccess once. Update the existing getErrors, getOverview, getTimeseries, getBreakdown, getErrorDetails, and getErrorSessions handlers to rely on the shared flow instead of duplicating logger.log and access checks, so the pattern is consistent with the other v2 controllers and harder to bypass accidentally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/v2/analytics-v2.service.ts`:
- Around line 1488-1501: The metadata lookup in getTrafficPagePropertyMetadata
bypasses the shared v2 timeframe normalization, so it can pass inconsistent
from/to/period values into the v1 analytics call. Update this path to use the
same normalization logic as the other v2 methods: treat from/to as an
all-or-nothing pair and fall back from period=custom to the default period when
bounds are missing, then pass the normalized period/timeBucket into
analyticsService.getCustomEventMetadata.
- Around line 1664-1670: The catch block in getFunnelStepDetails is logging the
raw reason object with console.error, which can leak query params and bypass the
gated logging behavior. Update analytics-v2.service.ts to avoid emitting the
unredacted error: either route the message through AppLoggerService.log without
force logging, or replace it with a redacted/non-sensitive message that
preserves context. Keep the fix localized to getFunnelStepDetails and ensure no
raw funnel query details are logged.
- Around line 751-771: The metric lookup in the performance timeseries branch
accepts inherited property names because it only checks truthiness on
PERFORMANCE_TIMESERIES_METRICS; update the dto.metrics handling in
analytics-v2.service.ts to use an own-property check before accepting a custom
metric name. Apply the same Object.hasOwn/hasOwnProperty guard pattern used
elsewhere in the timeseries code so names like constructor or toString are
rejected consistently in the performance, captcha, and errors branches, and keep
the existing UnprocessableEntityException path for unsupported metrics.
- Around line 1549-1564: The custom-event parsing in analytics-v2.service.ts
should reject the `__proto__` event name before it reaches the `series[event]`
assignment path, since that key mutates the plain object prototype and breaks
the timeseries response. Update the event normalization logic in the same block
that parses `dto.events` and validates `events.length` to filter out `__proto__`
while still allowing `constructor` and `prototype`, and keep the existing
`UnprocessableEntityException` behavior for empty results.
- Around line 1907-1921: The dimension handling in analytics-v2.service.ts is
routing virtual page dimensions into getErrorsFilters when type is 'errors', but
getErrorsFilters only supports ERROR_COLUMNS. Update the logic in the dimension
lookup flow around the existing getVersionFilters/getFilters branch so
dimension.virtual (entry_page and exit_page) is resolved through getFilters
before the errors-specific branch. Keep the existing dimensionApi/envelope
behavior intact and ensure the non-virtual path still uses getErrorsFilters only
for real error columns.
In `@backend/apps/cloud/src/analytics/v2/dto/v2-base.dto.ts`:
- Line 116: Remove the unused DTO exports from v2-base.dto by deleting
V2PaginationDto and V2MeasureDto if they are not referenced elsewhere, or
connect them to the intended controllers if they are meant to be part of the
API. Keep V2_MAX_BREAKDOWN_LIMIT unchanged since it is still consumed by
V2BreakdownDto and V2ListQueryDto, and make sure the remaining exports in the
DTO module are only the ones actually used.
In `@backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts`:
- Around line 20-24: The toIsoTimestamp mapper currently passes raw strings
directly into dayjs.tz, which can parse partial dates inconsistently. Update
toIsoTimestamp to detect partial inputs like YYYY-MM and YYYY and normalize them
or parse them explicitly with customParseFormat before calling dayjs.tz, using
the existing DEFAULT_TIMEZONE fallback and keeping the output format unchanged.
In `@backend/apps/cloud/src/analytics/v2/query/filters.translator.ts`:
- Around line 135-149: The filter parser in filters.translator.ts currently
allows a non-empty key on dimensions that are not keyed, which later gets
ignored by resolveColumn. Update the validation in the filter-building logic to
reject any provided key unless the selected dimension is included in
V2_KEYED_FILTER_DIMENSIONS, and keep the existing invalid-key checks for keyed
dimensions. Use the existing parse path around the V2Filter construction and the
dimension/key checks to return an UnprocessableEntityException with a clear
message when a key is supplied for a non-keyed dimension.
- Around line 9-16: `V2_FILTER_OPERATORS` and `V2FilterOperator` in
`filters.translator.ts` are being flagged as unused exports by CI; make them
internal to the module by removing the `export` keyword unless there is a real
external consumer. Update the declarations near `V2_FILTER_OPERATORS` and
`V2FilterOperator` so only the local translator logic in this file uses them.
In `@docs/content/docs/api/stats-v2.mdx`:
- Around line 1-439: The file is failing oxfmt formatting checks in CI, so the
documented API reference content needs to be reformatted to match the
formatter’s expected style. Run oxfmt locally on the docs file and commit the
resulting formatting-only changes; use the stats-v2.mdx content as the target
and ensure no semantic text changes are introduced.
---
Nitpick comments:
In `@backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts`:
- Around line 55-208: The controller methods in ErrorsV2Controller repeat the
same logging and access-check boilerplate in every handler, so consolidate that
cross-cutting logic into a shared NestJS mechanism such as a custom guard,
interceptor, or param decorator that can resolve pid, uid, and x-password and
call analyticsV2Service.assertReadAccess once. Update the existing getErrors,
getOverview, getTimeseries, getBreakdown, getErrorDetails, and getErrorSessions
handlers to rely on the shared flow instead of duplicating logger.log and access
checks, so the pattern is consistent with the other v2 controllers and harder to
bypass accidentally.
In `@backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts`:
- Line 18: V2_MAX_ENTITY_LIMIT is exported but not reused, causing an unused
export and duplicated hardcoded limits. Update the DTOs that cap entity/session
results to import and use V2_MAX_ENTITY_LIMIT where the 150 limit applies,
especially V2FunnelSessionsQueryDto in project.dto.ts, and make
V2ErrorSessionsQueryDto follow the same shared pattern if appropriate; otherwise
remove the export from entities.dto.ts if it is not intended to be shared. Keep
the limit/offset decorators aligned with the existing DTO classes so the shared
constant is the single source of truth.
In `@backend/apps/cloud/src/analytics/v2/dto/project.dto.ts`:
- Around line 18-36: The V2FunnelQueryDto currently documents that steps and
funnelId are mutually exclusive, but the request is still accepted when both are
present and getPagesArray() silently prefers funnelId; add explicit cross-field
validation on V2FunnelQueryDto (or equivalent 400 handling in the funnel query
flow) so invalid combinations fail fast, and make sure getPagesArray() no longer
relies on fallback precedence for conflicting or empty input.
In `@backend/apps/cloud/src/analytics/v2/dto/v2-base.dto.ts`:
- Around line 161-169: The sort field in the V2 base DTO is only checked as a
string, so malformed values are deferred to downstream parsing. Add a DTO-level
shape validation on the sort property in v2-base.dto.ts, near the sort field
decorators, by updating the V2BaseDto sort declaration to include a pattern
check that only allows field:direction with asc or desc. Keep the existing
ApiProperty, IsOptional, and IsString decorators, and use the sort property and
parseSortParam flow as the main reference points when locating the change.
In `@backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts`:
- Line 5: The import in timeseries.mapper is reaching into the user entity
internals for DEFAULT_TIMEZONE, which tightly couples analytics to that module
layout. Move DEFAULT_TIMEZONE to a shared/common constants location (or
re-export it from a shared entrypoint) and update the timeseries.mapper import
to use that shared symbol instead of the deep relative path. Keep the mapper
logic unchanged and use the existing DEFAULT_TIMEZONE identifier as the
migration target.
🪄 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: 2dbf616e-c0f7-4577-9daa-36c65991e231
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
backend/apps/cloud/src/analytics/analytics.module.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/protection/analytics-read.guard.tsbackend/apps/cloud/src/analytics/protection/analytics-read.util.tsbackend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/cloud/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/ioredis.stub.tsbackend/apps/cloud/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/timeseries.mapper.spec.tsbackend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/controllers/captcha-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/performance-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/profiles-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/sessions-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/analytics/v2/dto/performance.dto.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/src/analytics/v2/dto/v2-base.dto.tsbackend/apps/cloud/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/envelope.tsbackend/apps/cloud/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/dimensions.tsbackend/apps/cloud/src/analytics/v2/registry/index.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/analytics/v2/registry/types.tsbackend/apps/cloud/src/main.tsbackend/jest.v2.config.jsbackend/package.jsonbackend/tsconfig.build.jsondocs/content/docs/api/meta.jsondocs/content/docs/api/stats-v2.mdxdocs/content/docs/api/stats.mdx
Comment on lines
+20
to
+24
| export const toIsoTimestamp = (value: string, timezone: string): string => { | ||
| const zone = timezone || DEFAULT_TIMEZONE | ||
|
|
||
| return dayjs.tz(value, zone).format() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does dayjs 1.11.21 timezone plugin (dayjs.tz) reliably parse partial date strings like '2026-07' and '2026' without customParseFormat plugin?
💡 Result:
No, the Day.js 1.11.21 Timezone plugin (dayjs.tz) does not reliably parse partial date strings like '2026-07' or '2026' without the customParseFormat plugin [1][2]. Day.js is designed primarily to parse strings that follow the ISO 8601 format [3][2]. When you provide a string that is not a standard ISO 8601 date-time string—such as a partial date—Day.js falls back to the native JavaScript Date object constructor [2]. The behavior of the native Date constructor can vary across environments and does not guarantee consistent or correct parsing for partial strings [2]. To reliably parse non-standard or partial date strings in Day.js, you must extend Day.js with the customParseFormat plugin and explicitly provide the format string (e.g., 'YYYY-MM' or 'YYYY') [1][2]. In summary: 1. Standard ISO 8601 strings (e.g., '2026-07-07T12:00:00Z') generally work as expected with the Timezone plugin. 2. Partial strings are not natively supported by the core parsing logic and relying on the fallback behavior is discouraged because it is not robust or consistent [2]. 3. For partial dates, the customParseFormat plugin is the required solution to ensure predictable behavior [2].
Citations:
- 1: https://day.js.org/docs/en/plugin/custom-parse-format
- 2: [BUG] dayjs.tz do nothing iamkun/dayjs#1965
- 3: Parsing issue with timezone since 1.10.2 iamkun/dayjs#1318
Parse partial dates explicitly before dayjs.tz dayjs.tz(value, zone) can fall back to native Date for YYYY-MM / YYYY, so these timeseries labels may parse inconsistently across runtimes. Normalize partial inputs or parse them with customParseFormat first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts` around
lines 20 - 24, The toIsoTimestamp mapper currently passes raw strings directly
into dayjs.tz, which can parse partial dates inconsistently. Update
toIsoTimestamp to detect partial inputs like YYYY-MM and YYYY and normalize them
or parse them explicitly with customParseFormat before calling dayjs.tz, using
the existing DEFAULT_TIMEZONE fallback and keeping the output format unchanged.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/tsconfig.tsbuildinfo (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGenerated build artifact shouldn't be committed.
tsconfig.tsbuildinfois TypeScript's incremental-build cache, regenerated on every compile. Committing it causes needless diff churn on unrelated PRs and can go stale/conflict across branches. Recommend removing it from version control and adding**/tsconfig.tsbuildinfoto.gitignore.🧹 Suggested cleanup
+**/tsconfig.tsbuildinfoAdd this to the repo's
.gitignore, then rungit rm --cached docs/tsconfig.tsbuildinfo.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/tsconfig.tsbuildinfo` at line 1, The docs/tsconfig.tsbuildinfo file is a generated TypeScript incremental-build artifact that should not be committed to version control, as it causes unnecessary diff churn and can become stale or cause conflicts. Remove docs/tsconfig.tsbuildinfo from git tracking (e.g. via git rm --cached docs/tsconfig.tsbuildinfo) and add an entry for **/tsconfig.tsbuildinfo to the repository's .gitignore file so it is not re-added in future commits.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/tsconfig.tsbuildinfo`:
- Line 1: The docs/tsconfig.tsbuildinfo file is a generated TypeScript
incremental-build artifact that should not be committed to version control, as
it causes unnecessary diff churn and can become stale or cause conflicts. Remove
docs/tsconfig.tsbuildinfo from git tracking (e.g. via git rm --cached
docs/tsconfig.tsbuildinfo) and add an entry for **/tsconfig.tsbuildinfo to the
repository's .gitignore file so it is not re-added in future commits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7185d844-a4b5-4f85-9191-888343613b86
📒 Files selected for processing (9)
backend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/src/analytics/v2/dto/v2-base.dto.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/index.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/analytics/v2/registry/types.tsdocs/content/docs/api/stats-v2.mdxdocs/tsconfig.tsbuildinfo
💤 Files with no reviewable changes (1)
- backend/apps/cloud/src/analytics/v2/registry/index.ts
✅ Files skipped from review due to trivial changes (1)
- docs/content/docs/api/stats-v2.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/apps/cloud/src/analytics/v2/dto/project.dto.ts
- backend/apps/cloud/src/analytics/v2/analytics-v2.service.ts
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/pages/Project/View/components/FilterRowsEditor.tsx (1)
156-171: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve array-valued v2 filters.
filterToUrlValue()flattensvalue: ['US', 'CA']into'US,CA';filterRowsToFilters()then returns that scalar directly toonChange. Opening and applying the editor therefore changes a multi-value filter into a single unmatched value. Keep the original array in row state or introduce a lossless multi-value editor representation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/View/components/FilterRowsEditor.tsx` around lines 156 - 171, The filter row state created in filterRowsToFilters must preserve array-valued v2 filter values instead of passing the flattened result of filterToUrlValue directly to onChange. Update the createFilterRow path and its corresponding conversion logic to use a lossless representation for multi-value filters, while retaining scalar handling for single-value filters and preserving the original ['US', 'CA'] values through editor apply.
🧹 Nitpick comments (1)
web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx (1)
62-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
projectViewFiltersToV2helper.The same conversion (with identical comment) is defined in
AddAViewModal.tsx(there with an optionalview?param). Consider hoisting a single shared helper (e.g., intoprojectViewSegmentsoranalyticsUrl) to keep the legacy→v2 boundary logic in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx` around lines 62 - 68, Extract the duplicated projectViewFiltersToV2 conversion from ProjectViewHeaderActions and AddAViewModal into a shared project-view utility, preserving the legacy-filter comment and behavior. Update both callers to import and reuse the shared helper, supporting the optional view parameter used by AddAViewModal without changing unmappable-filter handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/community/src/analytics/v2/dto/entities.dto.ts`:
- Around line 82-83: Update the show_resolved transformation in the DTO so only
true and 'true' become true, only false and 'false' become false, and all other
values remain unchanged for `@IsBoolean`() to reject. Preserve the existing
boolean validation behavior for valid inputs.
In `@web/app/api/v2/endpoints.ts`:
- Around line 163-198: Update the endpoint builders getErrorDetails,
getErrorSessions, and getSessionDetails to encode the dynamic eid and psid path
segments before interpolation, matching the existing profileId pattern. Apply
the same encoding to the dimension path segment in the related endpoint, while
preserving the surrounding route structure and query handling.
In `@web/app/pages/Project/tabs/Funnels/FunnelsView.tsx`:
- Around line 179-183: Remove the `queryClient.invalidateQueries` call for the
`['v2', id, 'funnel-sessions']` key in the step-change handling near the funnel
analytics invalidation, since `SessionsDrawer` loads data imperatively; retain
the `['v2', id, 'funnel']` invalidation.
In `@web/app/pages/Project/tabs/Goals/GoalSettingsModal.tsx`:
- Around line 94-145: The goal submission flow currently drops filters whose
value is null before serializing conditions, causing edited goals to lose
unsupported filters. Update handleSubmit and the filterToCondition conversion
path so null-valued rows are rejected with a validation error or represented in
the GoalCondition model, rather than silently filtered out; preserve all
user-edited conditions in the submitted goal.
In `@web/app/pages/Project/View/components/Filters.tsx`:
- Around line 58-78: Update isFilterUrlEntry to normalize array-valued
filter.value into the same URL representation as rawValue before comparison,
while preserving null and scalar handling. Compare the normalized values so
array filters correctly match existing URL entries and support removal/toggling
without duplicates.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx`:
- Around line 2328-2329: Update the translation mapping for referrer_name in the
relevant mapping configuration so it uses the dedicated canonical
referrer/domain translation key instead of project.mapping.ref; keep referrer
mapped to the full-URL label project.mapping.ref.
In `@web/app/routes/api.v2`.$.ts:
- Around line 12-29: Update loader to reject path traversal segments in
params['*'] before calling serverFetch, including literal and encoded dot-dot
segments that URL parsing could normalize. Return the existing 400 Invalid v2
API path response for rejected paths, while preserving valid project path
handling and preventing password headers from being forwarded on traversal
attempts.
In `@web/app/utils/analyticsUrl.ts`:
- Around line 99-128: The URL filter serialization and parsing paths must
preserve array-valued V2Filter values. Update filterToUrlValue and
parseFiltersFromUrl so multi-value filters use a matching encoding/decoding
strategy, returning an array with null elements preserved when the URL
represents multiple values, while retaining existing scalar string/null
behavior.
---
Outside diff comments:
In `@web/app/pages/Project/View/components/FilterRowsEditor.tsx`:
- Around line 156-171: The filter row state created in filterRowsToFilters must
preserve array-valued v2 filter values instead of passing the flattened result
of filterToUrlValue directly to onChange. Update the createFilterRow path and
its corresponding conversion logic to use a lossless representation for
multi-value filters, while retaining scalar handling for single-value filters
and preserving the original ['US', 'CA'] values through editor apply.
---
Nitpick comments:
In `@web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx`:
- Around line 62-68: Extract the duplicated projectViewFiltersToV2 conversion
from ProjectViewHeaderActions and AddAViewModal into a shared project-view
utility, preserving the legacy-filter comment and behavior. Update both callers
to import and reuse the shared helper, supporting the optional view parameter
used by AddAViewModal without changing unmappable-filter handling.
🪄 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: e0f6aa05-9ad1-4255-a915-9f1595f3986d
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (109)
backend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/tsconfig.app.jsonbackend/apps/community/src/analytics/analytics.module.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/protection/analytics-read.guard.tsbackend/apps/community/src/analytics/protection/analytics-read.util.tsbackend/apps/community/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/community/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/community/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/community/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/community/src/analytics/v2/__tests__/ioredis.stub.tsbackend/apps/community/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/community/src/analytics/v2/__tests__/timeseries.mapper.spec.tsbackend/apps/community/src/analytics/v2/analytics-v2.service.tsbackend/apps/community/src/analytics/v2/controllers/captcha-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/performance-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/profiles-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/sessions-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/analytics/v2/dto/performance.dto.tsbackend/apps/community/src/analytics/v2/dto/project.dto.tsbackend/apps/community/src/analytics/v2/dto/v2-base.dto.tsbackend/apps/community/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/community/src/analytics/v2/mappers/envelope.tsbackend/apps/community/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/community/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/community/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/community/src/analytics/v2/query/filters.translator.tsbackend/apps/community/src/analytics/v2/registry/dimensions.tsbackend/apps/community/src/analytics/v2/registry/index.tsbackend/apps/community/src/analytics/v2/registry/metrics.tsbackend/apps/community/src/analytics/v2/registry/types.tsbackend/apps/community/tsconfig.app.jsonbackend/jest.v2.config.jsdocs/content/docs/api/stats-v2.mdxweb/app/api/api.server.tsweb/app/api/v2/client.tsweb/app/api/v2/endpoints.tsweb/app/api/v2/types.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/hooks/useInViewOnce.tsweb/app/hooks/v2/useV2Queries.tsweb/app/lib/constants/index.tsweb/app/lib/models/Entry.tsweb/app/lib/models/Project.tsweb/app/lib/v2Dimensions.tsweb/app/pages/Project/Settings/components/DeleteDataModal.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/AddAViewModal.tsxweb/app/pages/Project/View/components/FilterRowsEditor.tsxweb/app/pages/Project/View/components/Filters.tsxweb/app/pages/Project/View/components/LiveVisitorsDropdown.tsxweb/app/pages/Project/View/components/NoEvents.tsxweb/app/pages/Project/View/components/ProjectViewHeaderActions.tsxweb/app/pages/Project/View/components/SearchFilters.tsxweb/app/pages/Project/View/interfaces/traffic.tsweb/app/pages/Project/View/utils/filters.tsxweb/app/pages/Project/View/utils/projectViewSegments.tsweb/app/pages/Project/View/v2/BreakdownPanel.tsxweb/app/pages/Project/View/v2/TrafficMap.tsxweb/app/pages/Project/View/v2/adapters.tsweb/app/pages/Project/View/v2/loading.tsxweb/app/pages/Project/tabs/Captcha/CaptchaView.tsxweb/app/pages/Project/tabs/Captcha/NoCaptchaEvents.tsxweb/app/pages/Project/tabs/Captcha/adapters.tsweb/app/pages/Project/tabs/Errors/ErrorDetails.tsxweb/app/pages/Project/tabs/Errors/ErrorsMap.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Errors/types.tsweb/app/pages/Project/tabs/Experiments/ExperimentResults.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/Funnels/FunnelsList.tsxweb/app/pages/Project/tabs/Funnels/FunnelsView.tsxweb/app/pages/Project/tabs/Goals/GoalSettingsModal.tsxweb/app/pages/Project/tabs/Goals/GoalsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceMap.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Performance/perfAdapters.tsweb/app/pages/Project/tabs/Profiles/ProfileDetails.tsxweb/app/pages/Project/tabs/Profiles/Profiles.tsxweb/app/pages/Project/tabs/Profiles/ProfilesView.tsxweb/app/pages/Project/tabs/Profiles/components/NoProfiles.tsxweb/app/pages/Project/tabs/Replays/NoReplays.tsxweb/app/pages/Project/tabs/Replays/ReplaysView.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/pages/Project/tabs/Sessions/SessionDetailView.tsxweb/app/pages/Project/tabs/Sessions/Sessions.tsxweb/app/pages/Project/tabs/Sessions/SessionsView.tsxweb/app/pages/Project/tabs/Sessions/components/NoSessions.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/pages/Project/tabs/Traffic/UserFlow.tsxweb/app/providers/CurrentProjectProvider.tsxweb/app/root.tsxweb/app/routes/api.analytics.tsweb/app/routes/api.v2.$.tsweb/app/routes/projects.$id.tsxweb/app/ui/FilterValueInput.tsxweb/app/utils/analyticsUrl.tsweb/package.jsonweb/public/locales/en.json
💤 Files with no reviewable changes (4)
- web/app/lib/constants/index.ts
- web/app/lib/models/Entry.ts
- web/app/pages/Project/View/utils/filters.tsx
- web/app/routes/api.analytics.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/apps/cloud/src/analytics/v2/dto/project.dto.ts
- backend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.ts
- docs/content/docs/api/stats-v2.mdx
- backend/apps/cloud/src/analytics/v2/analytics-v2.service.ts
Comment on lines
+58
to
+78
| const isFilterUrlEntry = ( | ||
| filter: V2Filter, | ||
| rawKey: string, | ||
| rawValue: string, | ||
| ) => { | ||
| const parsed = parseFilterKey(rawKey) | ||
|
|
||
| if ( | ||
| !parsed || | ||
| parsed.dimension !== filter.dimension || | ||
| parsed.key !== filter.key || | ||
| parsed.operator !== filter.operator | ||
| ) { | ||
| return false | ||
| } | ||
|
|
||
| const normalisedValue = | ||
| rawValue === '' || rawValue === 'null' ? null : rawValue | ||
|
|
||
| return normalisedValue === (filter.value ?? null) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 'value\s*:\s*\[' web/app/pages/Project web/app/utils/analyticsUrl.ts
rg -nP -C3 'Array\.isArray\(filter\.value\)|\.value\.map\(' web/appRepository: Swetrix/swetrix
Length of output: 1315
isFilterUrlEntry still misses array-valued filters
filter.value can be an array, and this path compares it directly to a single string | null. For array filters, the comparison never matches, so remove/toggle logic can leave stale URL entries or add duplicates. Normalize the filter value to the same URL form before comparing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/pages/Project/View/components/Filters.tsx` around lines 58 - 78,
Update isFilterUrlEntry to normalize array-valued filter.value into the same URL
representation as rawValue before comparison, while preserving null and scalar
handling. Compare the normalized values so array filters correctly match
existing URL entries and support removal/toggling without duplicates.
Comment on lines
+2328
to
+2329
| referrer: t('project.mapping.ref'), | ||
| referrer_name: t('project.mapping.ref'), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the canonical-referrer label distinct.
referrer is the full URL, while referrer_name is the canonical root domain. Mapping both to project.mapping.ref makes filters indistinguishable; restore the dedicated referrer-name/domain translation key.
Proposed fix
- referrer_name: t('project.mapping.ref'),
+ referrer_name: t('project.mapping.refn'),📝 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
| referrer: t('project.mapping.ref'), | |
| referrer_name: t('project.mapping.ref'), | |
| referrer: t('project.mapping.ref'), | |
| referrer_name: t('project.mapping.refn'), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/pages/Project/View/ViewProject.helpers.tsx` around lines 2328 - 2329,
Update the translation mapping for referrer_name in the relevant mapping
configuration so it uses the dedicated canonical referrer/domain translation key
instead of project.mapping.ref; keep referrer mapped to the full-URL label
project.mapping.ref.
Comment on lines
+99
to
+128
| export const filterToUrlValue = (filter: V2Filter): string => { | ||
| if (Array.isArray(filter.value)) { | ||
| return filter.value.map((v) => v ?? 'null').join(',') | ||
| } | ||
| return filter.value ?? 'null' | ||
| } | ||
|
|
||
| /** Parses all filters from URL search params into the v2 filter shape */ | ||
| export const parseFiltersFromUrl = ( | ||
| searchParams: URLSearchParams, | ||
| ): V2Filter[] => { | ||
| const filters: V2Filter[] = [] | ||
|
|
||
| for (const [rawKey, rawValue] of searchParams.entries()) { | ||
| const parsed = parseFilterKey(rawKey) | ||
|
|
||
| if (!parsed) { | ||
| continue | ||
| } | ||
|
|
||
| filters.push({ | ||
| dimension: parsed.dimension, | ||
| operator: parsed.operator, | ||
| value: rawValue === '' || rawValue === 'null' ? null : rawValue, | ||
| ...(parsed.key ? { key: parsed.key } : {}), | ||
| }) | ||
| } | ||
|
|
||
| return filters | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether array-valued V2Filter is actually produced anywhere in the UI
rg -nP -C3 'value:\s*\[' --type=ts web/app | rg -i filter
rg -nP -C3 'Array\.isArray\(.*value' --type=ts web/appRepository: Swetrix/swetrix
Length of output: 5361
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the analytics URL helpers and the V2Filter type/usages
ast-grep outline web/app/utils/analyticsUrl.ts --view expanded || true
printf '\n--- analyticsUrl.ts ---\n'
sed -n '1,220p' web/app/utils/analyticsUrl.ts
printf '\n--- V2Filter references ---\n'
rg -n "type V2Filter|interface V2Filter|V2Filter" web/app -g'*.ts' -g'*.tsx'
printf '\n--- parseFiltersFromUrl / filterToUrlValue references ---\n'
rg -n "parseFiltersFromUrl|filterToUrlValue" web/app -g'*.ts' -g'*.tsx'Repository: Swetrix/swetrix
Length of output: 17056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the surrounding filter flow in the project view and any URL encode/decode helpers.
rg -n "parseFiltersFromUrl|filterToUrlValue|legacyFilterToV2|v2FilterToLegacy" web/app/routes web/app/pages web/app/utils -g'*.ts' -g'*.tsx' -A4 -B4Repository: Swetrix/swetrix
Length of output: 32840
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether filter.value arrays are ever constructed in the analytics filter flow.
rg -n "value:\s*\[|value\.map\(|Array\.isArray\(filter\.value\)|conditions" web/app/pages web/app/routes web/app/utils -g'*.ts' -g'*.tsx' -A3 -B3Repository: Swetrix/swetrix
Length of output: 12837
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- analyticsUrl.ts ---'
sed -n '1,220p' web/app/utils/analyticsUrl.ts
printf '\n%s\n' '--- V2Filter type and construction sites ---'
rg -n "type V2Filter|interface V2Filter|V2Filter" web/app -g'*.ts' -g'*.tsx' -A2 -B2
printf '\n%s\n' '--- URL helpers call sites ---'
rg -n "parseFiltersFromUrl|filterToUrlValue" web/app -g'*.ts' -g'*.tsx' -A3 -B3Repository: Swetrix/swetrix
Length of output: 50371
Array-valued filters need a matching decoder. V2Filter.value already allows arrays, but filterToUrlValue flattens them into comma-separated text and parseFiltersFromUrl always returns a single string/null. Any multi-value filter loses its shape on reload or shared links.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/utils/analyticsUrl.ts` around lines 99 - 128, The URL filter
serialization and parsing paths must preserve array-valued V2Filter values.
Update filterToUrlValue and parseFiltersFromUrl so multi-value filters use a
matching encoding/decoding strategy, returning an array with null elements
preserved when the URL represents multiple values, while retaining existing
scalar string/null behavior.
The v1 "live visitors over time" chart series was built on the old /log?includeConcurrency path, which the v2 traffic migration dropped. Re-add it as a first-class v2 timeseries metric: Backend (cloud + community): - register a `concurrency` traffic metric (timeseries-only, sessions-join) - whitelist it in TRAFFIC_TIMESERIES_METRICS and, when requested, pass includeConcurrency through to the v1 groupChartByTimeBucket delegation (the extra ClickHouse sweep-line query only runs on demand) Frontend: - pivotTrafficTimeseries now surfaces the concurrency series - TrafficView requests the concurrency metric and exposes the Live visitors toggle; the existing getSettings/getColumns machinery already renders it. Concurrency can't be segmented, so the toggle is disabled (with a tooltip) and auto-cleared when dashboard filters are applied Docs: document the concurrency metric on the v2 stats API page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/apps/community/src/common/constants.ts (1)
136-168: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep the v2 filter allowlist single-sourced.
V2_VIEW_FILTER_DIMENSIONSduplicatesanalytics/v2/registry/dimensions.ts, whileproject.serviceuses this copy as a persistence gate. A future dimension added to the registry but missed here will be silently dropped from saved views. Prefer a dependency-neutral shared definition, or add a parity test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/community/src/common/constants.ts` around lines 136 - 168, Make V2_VIEW_FILTER_DIMENSIONS single-sourced with the definitions in analytics/v2/registry/dimensions.ts by moving the shared allowlist to a dependency-neutral module and importing it from both consumers, or add a parity test that fails whenever the two lists diverge. Preserve the existing persistence-gate behavior while ensuring newly registered dimensions cannot be silently omitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/common/constants.ts`:
- Around line 114-142: Update V2_VIEW_FILTER_DIMENSIONS to include all
error-only dimensions supported by the v2 registry, including error_name,
error_message, and error_filename, so filterUnsupportedColumns preserves them
when saving views. Keep the allowlist aligned with the registry and retain all
existing dimensions.
In `@backend/apps/community/src/project/project.service.ts`:
- Around line 456-458: Add runtime array validation to the DTO field supplying
filters used by filterUnsupportedColumns: apply IsArray and an appropriate
ArrayMaxSize bound, reusing the project’s established validation limit if
available. Keep the existing unsupported-dimension filtering behavior unchanged.
In `@web/app/pages/Project/tabs/Goals/GoalSettingsModal.tsx`:
- Around line 137-143: The serialized goal conditions now use v2 dimension names
that route validation rejects. Update isGoalConditions and the downstream
condition contract to accept fields such as country, device, browser, referrer,
and utm_source, or translate them to the existing legacy names before
submission; keep validation and submitted condition fields consistent.
---
Nitpick comments:
In `@backend/apps/community/src/common/constants.ts`:
- Around line 136-168: Make V2_VIEW_FILTER_DIMENSIONS single-sourced with the
definitions in analytics/v2/registry/dimensions.ts by moving the shared
allowlist to a dependency-neutral module and importing it from both consumers,
or add a parity test that fails whenever the two lists diverge. Preserve the
existing persistence-gate behavior while ensuring newly registered dimensions
cannot be silently omitted.
🪄 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: d2cdba22-5f26-4061-aab5-af3051a17fdd
📒 Files selected for processing (59)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/common/constants.tsbackend/apps/cloud/src/experiment/experiment.controller.tsbackend/apps/cloud/src/goal/goal.controller.tsbackend/apps/cloud/src/project/dto/create-project-view.dto.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/community/src/analytics/v2/analytics-v2.service.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/analytics/v2/query/filters.translator.tsbackend/apps/community/src/analytics/v2/registry/metrics.tsbackend/apps/community/src/common/constants.tsbackend/apps/community/src/experiment/experiment.controller.tsbackend/apps/community/src/goal/goal.controller.tsbackend/apps/community/src/project/dto/create-project-view.dto.tsbackend/apps/community/src/project/gsc.service.tsbackend/apps/community/src/project/project.service.tsdocs/content/docs/api/stats-v2.mdxdocs/content/docs/api/stats.mdxweb/app/api/api.server.tsweb/app/api/v2/endpoints.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/lib/constants/index.tsweb/app/pages/Project/Settings/components/DeleteDataModal.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/AddAViewModal.tsxweb/app/pages/Project/View/components/ProjectViewHeaderActions.tsxweb/app/pages/Project/View/interfaces/traffic.tsweb/app/pages/Project/View/utils/projectViewSegments.tsweb/app/pages/Project/View/v2/BreakdownPanel.tsxweb/app/pages/Project/View/v2/adapters.tsweb/app/pages/Project/View/v2/loading.tsxweb/app/pages/Project/tabs/Captcha/CaptchaView.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/Funnels/FunnelsList.tsxweb/app/pages/Project/tabs/Funnels/FunnelsView.tsxweb/app/pages/Project/tabs/Goals/GoalSettingsModal.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/pages/Project/tabs/Sessions/SessionsView.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/routes/api.analytics.tsweb/app/routes/api.v2.$.tsweb/app/routes/projects.$id.tsxweb/app/utils/analyticsUrl.tsweb/public/locales/en.json
💤 Files with no reviewable changes (2)
- web/app/utils/analyticsUrl.ts
- web/app/pages/Project/tabs/SEO/seo-utils.ts
🚧 Files skipped from review as they are similar to previous changes (31)
- docs/content/docs/api/stats.mdx
- backend/apps/community/src/analytics/v2/tests/filters.translator.spec.ts
- backend/apps/cloud/src/analytics/analytics.service.ts
- backend/apps/cloud/src/analytics/v2/registry/metrics.ts
- backend/apps/community/src/analytics/analytics.service.ts
- backend/apps/community/src/analytics/v2/registry/metrics.ts
- web/app/routes/api.v2.$.ts
- web/app/pages/Project/tabs/Sessions/SessionsView.tsx
- web/app/pages/Project/tabs/Funnels/FunnelsList.tsx
- web/app/lib/constants/index.ts
- web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx
- web/app/pages/Project/View/interfaces/traffic.ts
- web/app/pages/Project/tabs/SEO/SEOView.tsx
- web/app/pages/Project/View/ViewProject.helpers.tsx
- backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts
- web/app/pages/Project/View/v2/BreakdownPanel.tsx
- docs/content/docs/api/stats-v2.mdx
- web/app/pages/Project/tabs/Funnels/FunnelsView.tsx
- web/app/pages/Project/tabs/Captcha/CaptchaView.tsx
- web/app/pages/Project/View/v2/adapters.ts
- backend/apps/community/src/analytics/v2/dto/entities.dto.ts
- web/app/api/v2/endpoints.ts
- web/app/pages/Project/tabs/Traffic/TrafficView.tsx
- web/app/pages/Project/View/ViewProject.tsx
- web/app/pages/Project/tabs/Errors/ErrorsView.tsx
- web/app/routes/api.analytics.ts
- web/app/pages/Project/tabs/Performance/PerformanceView.tsx
- web/app/hooks/useAnalyticsProxy.ts
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
- backend/apps/community/src/analytics/v2/analytics-v2.service.ts
- backend/apps/cloud/src/analytics/v2/analytics-v2.service.ts
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/app/pages/Project/View/utils/projectViewSegments.ts (1)
39-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
queryfor SEO filters
SEO_FILTER_COLUMNSandFILTER_OPTIONS_BY_TAB[seo]still usekeywords, but SEO v2 emitsquery. That makesqueryfilters from SEO breakdown links look unsupported when saving/applying views, and the manual picker never offers them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/View/utils/projectViewSegments.ts` at line 39, Update SEO filter configuration to use query instead of keywords: replace keywords in SEO_FILTER_COLUMNS and the seo entry of FILTER_OPTIONS_BY_TAB so SEO breakdown links, saved views, and the manual picker all recognize and offer query filters.backend/apps/cloud/src/project/dto/create-project-view.dto.ts (1)
74-83: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale Swagger description for
filters.The description still documents the pre-V2 shape (
{ column, filter, isExclusive, isContains }), butFilterwas changed to{ dimension, operator, value, key? }. This could mislead API consumers reading the generated docs.✏️ Suggested fix
`@ApiProperty`({ description: - 'An array of properties to filter [{ column, filter, isExclusive, isContains }]', + 'An array of properties to filter [{ dimension, operator, value, key? }]', required: false, isArray: true, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts` around lines 74 - 83, Update the `filters` `@ApiProperty` description in the create-project view DTO to document the current `Filter` shape `{ dimension, operator, value, key? }` instead of the obsolete V2 fields, while preserving the existing optional, array, and size validation.backend/apps/cloud/src/analytics/analytics.controller.ts (1)
1016-1079: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winCap
stepsandjourneysingetJourneys.getSafeNumberonly normalizes the inputs; it doesn’t bound them, so a client can still trigger an expensive ClickHouse query. Add an explicit maximum here, or clamp ingetSafeNumber, before callinggetJourneys.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.controller.ts` around lines 1016 - 1079, Cap the `steps` and `journeys` values in `getJourneys` after `getSafeNumber` normalizes them and before calling `analyticsService.getJourneys`. Enforce explicit maximum limits for both client-controlled values while preserving their existing defaults and downstream query flow.backend/apps/community/src/project/gsc.service.ts (1)
570-623: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winArray-valued GSC filters are silently ignored in
backend/apps/community/src/project/gsc.service.ts:570-623.Filter.valueallows(string | null)[], but this parser only accepts strings, so multi-value filters disappear without warning. Reject them explicitly or translate them into supported Search Console filters before building the request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/community/src/project/gsc.service.ts` around lines 570 - 623, Update the filter parsing logic around the raw value extraction in the GSC filter builder to explicitly handle array-valued Filter.value inputs instead of silently skipping them through the string type check. Reject unsupported arrays with clear handling, or translate them into equivalent supported Search Console filters before constructing the request, while preserving existing behavior for scalar string values.
🧹 Nitpick comments (1)
backend/apps/cloud/src/analytics/analytics.service.ts (1)
1143-1230: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider capping
groupArray((created, pg))ingetJourneysto guard against pathological sessions. Both the cloud and communitygetJourneysimplementations build an unbounded per-session array of every matching pageview before compacting/truncating it tosteps; a session with an unusually large pageview count (retry storms, bot-detection bypass, etc.) would materialize a correspondingly large array for that one group.
backend/apps/cloud/src/analytics/analytics.service.ts#L1143-L1230: cap the array collected insession_paths(e.g. via ClickHouse'sgroupArray(max_size)(x)variant) if a bound is desired.backend/apps/community/src/analytics/analytics.service.ts#L946-L1033: apply the same cap here.Any cap must be applied carefully:
groupArray(N)truncates before the subsequentarraySort, and per ClickHouse docs the retained N elements are "the first N encountered during processing – not necessarily the first N by time," so naively capping here could silently return a chronologically-wrong path instead of the true first-N pageviews. A correct fix would need an order-preserving variant (e.g.groupArraySorted, if available on the ClickHouse version in use) rather than a plaingroupArray(N)swap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 1143 - 1230, Update getJourneys in backend/apps/cloud/src/analytics/analytics.service.ts (1143-1230) and backend/apps/community/src/analytics/analytics.service.ts (946-1033) to bound per-session pageview aggregation using an order-preserving ClickHouse aggregate, such as the supported groupArraySorted variant, rather than plain groupArray(N). Ensure the retained entries still represent the earliest pageviews by created time before arrayCompact and truncation to steps, preserving chronological journey paths while preventing unbounded arrays.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/docs/api/stats-v2.mdx`:
- Line 62: Update the traffic metric entry in the stats-v2 documentation table
so its concurrency description states that filtered requests return zero-filled
values, matching the behavior documented near the concurrency details. Preserve
the existing scope note that concurrency is for live visitors over time and
timeseries only.
In `@web/app/hooks/v2/useV2Queries.ts`:
- Around line 218-229: Update getNextPageParam to return undefined immediately
when lastPage.data is empty, before applying the total-based or full-page
fallback logic. Preserve the existing pagination behavior for non-empty pages
while ensuring zero-record responses cannot return the unchanged loaded offset.
---
Outside diff comments:
In `@backend/apps/cloud/src/analytics/analytics.controller.ts`:
- Around line 1016-1079: Cap the `steps` and `journeys` values in `getJourneys`
after `getSafeNumber` normalizes them and before calling
`analyticsService.getJourneys`. Enforce explicit maximum limits for both
client-controlled values while preserving their existing defaults and downstream
query flow.
In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts`:
- Around line 74-83: Update the `filters` `@ApiProperty` description in the
create-project view DTO to document the current `Filter` shape `{ dimension,
operator, value, key? }` instead of the obsolete V2 fields, while preserving the
existing optional, array, and size validation.
In `@backend/apps/community/src/project/gsc.service.ts`:
- Around line 570-623: Update the filter parsing logic around the raw value
extraction in the GSC filter builder to explicitly handle array-valued
Filter.value inputs instead of silently skipping them through the string type
check. Reject unsupported arrays with clear handling, or translate them into
equivalent supported Search Console filters before constructing the request,
while preserving existing behavior for scalar string values.
In `@web/app/pages/Project/View/utils/projectViewSegments.ts`:
- Line 39: Update SEO filter configuration to use query instead of keywords:
replace keywords in SEO_FILTER_COLUMNS and the seo entry of
FILTER_OPTIONS_BY_TAB so SEO breakdown links, saved views, and the manual picker
all recognize and offer query filters.
---
Nitpick comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 1143-1230: Update getJourneys in
backend/apps/cloud/src/analytics/analytics.service.ts (1143-1230) and
backend/apps/community/src/analytics/analytics.service.ts (946-1033) to bound
per-session pageview aggregation using an order-preserving ClickHouse aggregate,
such as the supported groupArraySorted variant, rather than plain groupArray(N).
Ensure the retained entries still represent the earliest pageviews by created
time before arrayCompact and truncation to steps, preserving chronological
journey paths while preventing unbounded arrays.
🪄 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: 9b2a74a3-af59-4b1a-94ba-f05776c578e8
📒 Files selected for processing (132)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.module.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/interfaces/index.tsbackend/apps/cloud/src/analytics/protection/analytics-read.util.tsbackend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/cloud/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/ioredis.stub.tsbackend/apps/cloud/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/src/analytics/v2/dto/seo.dto.tsbackend/apps/cloud/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/dimensions.tsbackend/apps/cloud/src/analytics/v2/registry/index.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/analytics/v2/registry/seo.tsbackend/apps/cloud/src/analytics/v2/registry/types.tsbackend/apps/cloud/src/analytics/v2/seo-v2.service.tsbackend/apps/cloud/src/common/constants.tsbackend/apps/cloud/src/experiment/experiment.controller.tsbackend/apps/cloud/src/goal/goal.controller.tsbackend/apps/cloud/src/project/dto/create-project-view.dto.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.module.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/interfaces/index.tsbackend/apps/community/src/analytics/protection/analytics-read.guard.tsbackend/apps/community/src/analytics/protection/analytics-read.util.tsbackend/apps/community/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/community/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/community/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/community/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/community/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/community/src/analytics/v2/analytics-v2.service.tsbackend/apps/community/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/analytics/v2/dto/project.dto.tsbackend/apps/community/src/analytics/v2/dto/seo.dto.tsbackend/apps/community/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/community/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/community/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/community/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/community/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/community/src/analytics/v2/query/filters.translator.tsbackend/apps/community/src/analytics/v2/registry/dimensions.tsbackend/apps/community/src/analytics/v2/registry/index.tsbackend/apps/community/src/analytics/v2/registry/metrics.tsbackend/apps/community/src/analytics/v2/registry/seo.tsbackend/apps/community/src/analytics/v2/registry/types.tsbackend/apps/community/src/analytics/v2/seo-v2.service.tsbackend/apps/community/src/common/constants.tsbackend/apps/community/src/experiment/experiment.controller.tsbackend/apps/community/src/goal/goal.controller.tsbackend/apps/community/src/project/dto/create-project-view.dto.tsbackend/apps/community/src/project/gsc.service.tsbackend/apps/community/src/project/project.service.tsdocs/content/docs/api/stats-v2.mdxdocs/content/docs/api/stats.mdxweb/app/api/api.server.tsweb/app/api/v2/client.tsweb/app/api/v2/endpoints.tsweb/app/api/v2/types.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/hooks/useInViewOnce.tsweb/app/hooks/v2/useV2Queries.tsweb/app/lib/constants/index.tsweb/app/lib/v2Dimensions.tsweb/app/pages/Project/Settings/components/DeleteDataModal.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/AddAViewModal.tsxweb/app/pages/Project/View/components/FilterRowsEditor.tsxweb/app/pages/Project/View/components/Filters.tsxweb/app/pages/Project/View/components/ProjectViewHeaderActions.tsxweb/app/pages/Project/View/interfaces/traffic.tsweb/app/pages/Project/View/utils/projectViewSegments.tsweb/app/pages/Project/View/v2/BreakdownPanel.tsxweb/app/pages/Project/View/v2/TrafficMap.tsxweb/app/pages/Project/View/v2/adapters.tsweb/app/pages/Project/View/v2/loading.tsxweb/app/pages/Project/tabs/Captcha/CaptchaView.tsxweb/app/pages/Project/tabs/Captcha/adapters.tsweb/app/pages/Project/tabs/Errors/ErrorsMap.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Errors/types.tsweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/Funnels/FunnelsList.tsxweb/app/pages/Project/tabs/Funnels/FunnelsView.tsxweb/app/pages/Project/tabs/Goals/GoalSettingsModal.tsxweb/app/pages/Project/tabs/Goals/GoalsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceMap.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Performance/perfAdapters.tsweb/app/pages/Project/tabs/Profiles/ProfilesView.tsxweb/app/pages/Project/tabs/SEO/CompactReferralPanel.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/pages/Project/tabs/Sessions/SessionsView.tsxweb/app/pages/Project/tabs/Traffic/MetricCards.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/root.tsxweb/app/routes/api.analytics.tsweb/app/routes/api.v2.$.tsweb/app/routes/projects.$id.tsxweb/app/ui/FilterValueInput.tsxweb/app/ui/NumberFlow.tsxweb/app/utils/analyticsUrl.tsweb/public/locales/en.json
💤 Files with no reviewable changes (40)
- backend/apps/cloud/src/analytics/v2/tests/ioredis.stub.ts
- backend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.ts
- web/app/pages/Project/View/v2/loading.tsx
- backend/apps/cloud/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- backend/apps/community/src/analytics/protection/cacheable-analytics.decorator.ts
- backend/apps/community/src/analytics/v2/registry/types.ts
- web/app/pages/Project/tabs/Captcha/adapters.ts
- web/app/pages/Project/tabs/Performance/PerformanceMap.tsx
- backend/apps/community/src/analytics/v2/query/breakdown-query.builder.ts
- web/app/root.tsx
- backend/apps/community/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- web/app/pages/Project/View/v2/TrafficMap.tsx
- backend/apps/cloud/src/analytics/v2/mappers/entity.mapper.ts
- backend/apps/community/src/analytics/v2/mappers/summary.mapper.ts
- backend/apps/community/src/analytics/v2/registry/index.ts
- backend/apps/community/src/analytics/v2/mappers/entity.mapper.ts
- web/app/hooks/useInViewOnce.ts
- backend/apps/cloud/src/analytics/v2/tests/registry.spec.ts
- web/app/pages/Project/tabs/Performance/perfAdapters.ts
- web/app/pages/Project/tabs/Errors/types.ts
- backend/apps/community/src/analytics/v2/mappers/timeseries.mapper.ts
- backend/apps/cloud/src/analytics/v2/registry/index.ts
- web/app/pages/Project/tabs/Errors/ErrorsMap.tsx
- backend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.ts
- backend/apps/cloud/src/analytics/v2/registry/dimensions.ts
- backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts
- backend/apps/cloud/src/analytics/v2/mappers/summary.mapper.ts
- backend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.ts
- backend/apps/cloud/src/analytics/v2/registry/types.ts
- backend/apps/community/src/analytics/v2/registry/dimensions.ts
- backend/apps/community/src/analytics/v2/controllers/errors-v2.controller.ts
- web/app/pages/Project/tabs/Profiles/ProfilesView.tsx
- backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts
- web/app/pages/Project/View/components/Filters.tsx
- web/app/utils/analyticsUrl.ts
- web/app/pages/Project/View/components/FilterRowsEditor.tsx
- backend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.ts
- backend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.ts
- web/app/ui/FilterValueInput.tsx
- web/app/pages/Project/tabs/Funnels/FunnelsView.tsx
🚧 Files skipped from review as they are similar to previous changes (48)
- backend/apps/cloud/src/common/constants.ts
- backend/apps/cloud/src/analytics/analytics.module.ts
- backend/apps/cloud/src/project/project.service.ts
- backend/apps/community/src/common/constants.ts
- web/app/routes/api.v2.$.ts
- backend/apps/community/src/experiment/experiment.controller.ts
- web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx
- web/public/locales/en.json
- web/app/pages/Project/Settings/components/DeleteDataModal.tsx
- backend/apps/community/src/project/dto/create-project-view.dto.ts
- backend/apps/community/src/analytics/protection/public-project-cache.interceptor.ts
- web/app/pages/Project/View/components/AddAViewModal.tsx
- backend/apps/community/src/analytics/analytics.module.ts
- web/app/lib/v2Dimensions.ts
- backend/apps/community/src/analytics/v2/tests/filters.translator.spec.ts
- backend/apps/community/src/analytics/v2/tests/registry.spec.ts
- backend/apps/community/src/goal/goal.controller.ts
- backend/apps/community/src/analytics/protection/analytics-read.util.ts
- backend/apps/community/src/analytics/v2/dto/project.dto.ts
- web/app/pages/Project/View/interfaces/traffic.ts
- backend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.ts
- web/app/api/v2/client.ts
- backend/apps/community/src/analytics/v2/dto/entities.dto.ts
- backend/apps/cloud/src/analytics/v2/dto/project.dto.ts
- backend/apps/community/src/analytics/v2/controllers/project-v2.controller.ts
- web/app/pages/Project/View/v2/adapters.ts
- backend/apps/cloud/src/analytics/v2/query/filters.translator.ts
- web/app/pages/Project/tabs/Goals/GoalSettingsModal.tsx
- web/app/pages/Project/tabs/Funnels/FunnelsList.tsx
- backend/apps/cloud/src/analytics/v2/registry/metrics.ts
- web/app/pages/Project/View/ViewProject.helpers.tsx
- backend/apps/community/src/analytics/protection/analytics-read.guard.ts
- backend/apps/cloud/src/goal/goal.controller.ts
- web/app/pages/Project/tabs/Sessions/SessionsView.tsx
- web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx
- backend/apps/cloud/src/analytics/v2/tests/filters.translator.spec.ts
- web/app/pages/Project/View/v2/BreakdownPanel.tsx
- backend/apps/community/src/analytics/v2/registry/metrics.ts
- backend/apps/community/src/analytics/v2/query/filters.translator.ts
- web/app/routes/projects.$id.tsx
- web/app/pages/Project/tabs/Performance/PerformanceView.tsx
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
- backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts
- web/app/pages/Project/tabs/Traffic/TrafficView.tsx
- web/app/pages/Project/tabs/Captcha/CaptchaView.tsx
- web/app/pages/Project/View/Panels.tsx
- web/app/api/api.server.ts
- web/app/pages/Project/tabs/Errors/ErrorsView.tsx
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/apps/cloud/src/project/dto/create-project-view.dto.ts (1)
74-79: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale Swagger description.
The
@ApiPropertydescription still documents the old v1 filter shape{ column, filter, isExclusive, isContains }, butFilteris now{ dimension, operator, value, key? }. This misleads API consumers reading the generated docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts` around lines 74 - 79, Update the description in the `@ApiProperty` decorator for the filter array to document the current Filter shape `{ dimension, operator, value, key? }` instead of the obsolete v1 fields, while preserving the existing optional array metadata.backend/apps/cloud/src/analytics/analytics.service.ts (1)
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getConcurrencyChartDatalacks error handling in both apps, so a query failure fails the whole traffic-chart response. Unlike sibling best-effort ClickHouse helpers in the same files (getOnlineUserCount,checkSessionExistsInClickHouse,getSessionDurationFromClickHouse), this new method has notry/catch; any failure propagates through thePromise.allingroupChartByTimeBucketand takes down the entire/analyticstraffic response instead of degrading to zeroed concurrency.
backend/apps/cloud/src/analytics/analytics.service.ts#L5865-5881: wrap the query intry/catch, returningArray(xShifted.length).fill(0)on error (matching the sibling helpers' fallback pattern).backend/apps/community/src/analytics/analytics.service.ts#L4413-4429: apply the same fix here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` at line 1, Add try/catch handling to getConcurrencyChartData in both analytics service implementations, returning a zero-filled array sized to xShifted.length when the ClickHouse query fails. Preserve the existing successful query result and ensure groupChartByTimeBucket receives the fallback instead of a propagated error.
🧹 Nitpick comments (8)
web/app/ui/NumberFlow.tsx (1)
156-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConditionally set
maximumFractionDigitsfor cleaner integer animations.When
showMsisfalse,srepresents a whole number. PassingmaximumFractionDigits: 2unconditionally might cause the component to interpolate through decimal values during the animation transition. To ensure the counter rolls purely as whole numbers when milliseconds are hidden, consider conditioning this format property.✨ Proposed optional refactor
{showS ? ( <NumberFlow className={FLOW_VALUE_CLASS} {...FLOW_TIMING} value={s} suffix='s' - format={{ maximumFractionDigits: 2 }} + format={{ maximumFractionDigits: showMs ? 2 : 0 }} willChange /> ) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/ui/NumberFlow.tsx` around lines 156 - 166, Update the seconds NumberFlow instance in the showS branch so its format conditionally sets maximumFractionDigits to 0 when showMs is false and retains 2 when milliseconds are shown, ensuring hidden-millisecond animations remain whole-number based.web/app/api/v2/client.ts (1)
3-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExport
V2ApiErrorfor downstream status-based handling.
fetchV2throwsV2ApiError, but the class isn't exported, so consumers can'tinstanceof-check it or type-safely read.status(e.g., to special-case 404/429 in the v2 query hooks or tab views). Since this cohort explicitly introducesV2ApiErroras part of the client's public contract, exporting it costs nothing and unlocks structured error handling downstream.♻️ Proposed fix
-class V2ApiError extends Error { +export class V2ApiError extends Error { status: number🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/api/v2/client.ts` around lines 3 - 11, Export the V2ApiError class so downstream consumers can use instanceof checks and access its status property when handling errors from fetchV2. Keep the existing constructor, name, and status behavior unchanged.backend/apps/cloud/src/project/dto/create-project-view.dto.ts (1)
50-55: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd nested validation for persisted
Filteritems.
filters?: Filter[]only checks array-ness and length; nothing validates that each item'sdimension/operator/valueconform to the new v2 shape (e.g.operatorrestricted to'is'|'is_not'|'contains'|'contains_not'). Malformed filters could be persisted and later mis-handled by query-building code that assumes well-formed values.♻️ Suggested approach
+class ProjectViewFilterDto implements Filter { + `@IsString`() + dimension: string + + `@IsIn`(['is', 'is_not', 'contains', 'contains_not']) + operator: 'is' | 'is_not' | 'contains' | 'contains_not' + + // validate string | null | (string|null)[] as appropriate + value: string | null | (string | null)[] + + `@IsOptional`() + `@IsString`() + key?: string +} ... `@IsOptional`() `@IsArray`() `@ArrayMaxSize`(MAX_FILTERS_IN_VIEW) - filters?: Filter[] + `@ValidateNested`({ each: true }) + `@Type`(() => ProjectViewFilterDto) + filters?: ProjectViewFilterDto[]Also applies to: 81-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts` around lines 50 - 55, Update the Filter definition and the filters property in the containing DTO to perform nested validation for every persisted filter item. Ensure each item validates dimension as a string, operator against the four allowed literals, and value according to its string/null or array-of-string/null shape, while preserving the existing optional array and length constraints.backend/apps/cloud/src/analytics/analytics.service.ts (2)
140-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
MAX_CONCURRENCY_WINDOW_MINUTES(366 days) is very generous for a "live visitors" feature.The sweep-line query in
generateConcurrencyAggregationQuerymaterializes one row per minute of the window vianumbers(...). With this cap, a request combiningincludeConcurrency=truewith a large period (e.g.all/365d) can generate ~527K rows per request. Since this feature is scoped to the live-visitors chart, a much tighter cap (hours/days, not a year) would better match actual usage while bounding worst-case query cost. See consolidated comment for the duplicate in the community app.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 140 - 142, The MAX_CONCURRENCY_WINDOW_MINUTES cap is too large for the live-visitors concurrency query and can materialize hundreds of thousands of rows. Reduce this constant to a tight hours-or-days limit appropriate for the live-visitors chart, while leaving generateConcurrencyAggregationQuery and other period handling unchanged.
1-1: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
MAX_CONCURRENCY_WINDOW_MINUTES(366 days) is too generous for a live-visitors feature, duplicated in both apps. The sweep-line query materializes one row per minute of the requested window vianumbers(...); at this cap a single request can generate ~527K rows, with no other server-side guard limiting how large a window can be combined withincludeConcurrency=true.
backend/apps/cloud/src/analytics/analytics.service.ts#L140-142: lower the cap to match the live-visitors use case (hours/days, not a year).backend/apps/community/src/analytics/analytics.service.ts#L123-125: apply the same lowered cap here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` at line 1, Lower the MAX_CONCURRENCY_WINDOW_MINUTES cap in both analytics service implementations, backend/apps/cloud and backend/apps/community, to a hours/days-scale limit appropriate for live visitors instead of 366 days. Keep the same constant name and ensure both applications use the identical reduced value to bound includeConcurrency requests and sweep-line row generation.backend/apps/cloud/src/analytics/v2/seo-v2.service.ts (1)
267-296: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getSummarycomputes a meaningless "previous" period forperiod=all.Unlike
AnalyticsV2Service.getCaptchaSummary, which explicitly skips the previous-period computation whentimeframe.period === 'all', this always fetches a second full-length GSC window. Forperiod=allthat previous window sits well outside Search Console's ~16-month retention (per this file's own doc/comment about ~16-month retention), so it either burns an extra expensive GSC quota call for nothing or produces a misleadingchangecomparison against near-empty data.♻️ Suggested fix
const [current, previous] = await Promise.all([ this.gscService.getSummary(pid, groupFrom, groupTo, gscFilters, ctx), - this.gscService - .getSummary(pid, previousFrom, previousTo, gscFilters, ctx) - .catch(() => null), + timeframe.period === 'all' + ? Promise.resolve(null) + : this.gscService + .getSummary(pid, previousFrom, previousTo, gscFilters, ctx) + .catch(() => null), ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/seo-v2.service.ts` around lines 267 - 296, Update getSummary to skip the previous-period GSC request when timeframe.period is 'all', passing null as the previous summary in that case. Preserve the existing previous-window calculation and error handling for all other periods.docs/content/docs/api/stats-v2.mdx (1)
247-252: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
concurrencyis missing from the "not computable per dimension" note.Line 250 calls out
session_duration/bounce_rateas excluded from/traffic/breakdown, butconcurrency(documented earlier as session-interval-derived and timeseries-only) isn't mentioned here at all. This lines up withAnalyticsV2Service.getBreakdownin the backend also lacking a runtime guard againstmetrics=concurrency— likely the same oversight on both sides.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/api/stats-v2.mdx` around lines 247 - 252, Update the “not computable per dimension” note in the stats-v2 documentation to include concurrency alongside session_duration and bounce_rate, stating that it must be requested through timeseries or summary endpoints. Also add the corresponding runtime validation in AnalyticsV2Service.getBreakdown to reject concurrency requests for breakdown dimensions.web/app/pages/Project/tabs/SEO/SEOView.tsx (1)
245-249: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGate
referrerQueryonisConnectedlike the other SEO queries.
referrerQuery(used forsearchEngineEntries/aiReferralEntriesviaCompactReferralPanel) has noenabledcondition, unlike every other query in this component. Its results are only rendered in the JSX path reached after the!isConnectedearly return, so when GSC isn't connected this still fires a network request whose data is never displayed.♻️ Proposed fix
const referrerQuery = useBreakdownQuery('traffic', { dimension: 'referrer', limit: 100, sort: 'visitors:desc', + enabled: isConnected, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/SEO/SEOView.tsx` around lines 245 - 249, Update the referrerQuery useBreakdownQuery call in SEOView to include the same isConnected-based enabled condition used by the other SEO queries, preventing the request when the component is disconnected while preserving its current query parameters and rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 5811-5882: Update getConcurrencyChartData to wrap the ClickHouse
query and result extraction in try/catch, matching the graceful-failure pattern
used by getOnlineUserCount, checkSessionExistsInClickHouse, and
getSessionDurationFromClickHouse. On any query or parsing error, log the failure
using the service’s established logging approach and return the appropriate
empty concurrency result so groupChartByTimeBucket continues successfully
without failing the entire traffic chart.
In `@backend/apps/community/src/analytics/analytics.service.ts`:
- Around line 4353-4430: Add the same error-handling behavior used by the cloud
app to getConcurrencyChartData around the ClickHouse query and result parsing,
ensuring failures are caught and handled consistently while preserving the
existing successful return through extractCountsForSessionChart.
In `@backend/apps/community/src/analytics/v2/analytics-v2.service.ts`:
- Line 78: Update the community analytics v2 getBreakdown flow and its related
handling around the referenced metric configuration so requests using
metrics=concurrency are explicitly rejected on /traffic/breakdown, matching the
guard behavior in the cloud analytics service and existing non-per-row metric
validation.
---
Outside diff comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Line 1: Add try/catch handling to getConcurrencyChartData in both analytics
service implementations, returning a zero-filled array sized to xShifted.length
when the ClickHouse query fails. Preserve the existing successful query result
and ensure groupChartByTimeBucket receives the fallback instead of a propagated
error.
In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts`:
- Around line 74-79: Update the description in the `@ApiProperty` decorator for
the filter array to document the current Filter shape `{ dimension, operator,
value, key? }` instead of the obsolete v1 fields, while preserving the existing
optional array metadata.
---
Nitpick comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 140-142: The MAX_CONCURRENCY_WINDOW_MINUTES cap is too large for
the live-visitors concurrency query and can materialize hundreds of thousands of
rows. Reduce this constant to a tight hours-or-days limit appropriate for the
live-visitors chart, while leaving generateConcurrencyAggregationQuery and other
period handling unchanged.
- Line 1: Lower the MAX_CONCURRENCY_WINDOW_MINUTES cap in both analytics service
implementations, backend/apps/cloud and backend/apps/community, to a
hours/days-scale limit appropriate for live visitors instead of 366 days. Keep
the same constant name and ensure both applications use the identical reduced
value to bound includeConcurrency requests and sweep-line row generation.
In `@backend/apps/cloud/src/analytics/v2/seo-v2.service.ts`:
- Around line 267-296: Update getSummary to skip the previous-period GSC request
when timeframe.period is 'all', passing null as the previous summary in that
case. Preserve the existing previous-window calculation and error handling for
all other periods.
In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts`:
- Around line 50-55: Update the Filter definition and the filters property in
the containing DTO to perform nested validation for every persisted filter item.
Ensure each item validates dimension as a string, operator against the four
allowed literals, and value according to its string/null or array-of-string/null
shape, while preserving the existing optional array and length constraints.
In `@docs/content/docs/api/stats-v2.mdx`:
- Around line 247-252: Update the “not computable per dimension” note in the
stats-v2 documentation to include concurrency alongside session_duration and
bounce_rate, stating that it must be requested through timeseries or summary
endpoints. Also add the corresponding runtime validation in
AnalyticsV2Service.getBreakdown to reject concurrency requests for breakdown
dimensions.
In `@web/app/api/v2/client.ts`:
- Around line 3-11: Export the V2ApiError class so downstream consumers can use
instanceof checks and access its status property when handling errors from
fetchV2. Keep the existing constructor, name, and status behavior unchanged.
In `@web/app/pages/Project/tabs/SEO/SEOView.tsx`:
- Around line 245-249: Update the referrerQuery useBreakdownQuery call in
SEOView to include the same isConnected-based enabled condition used by the
other SEO queries, preventing the request when the component is disconnected
while preserving its current query parameters and rendering behavior.
In `@web/app/ui/NumberFlow.tsx`:
- Around line 156-166: Update the seconds NumberFlow instance in the showS
branch so its format conditionally sets maximumFractionDigits to 0 when showMs
is false and retains 2 when milliseconds are shown, ensuring hidden-millisecond
animations remain whole-number based.
🪄 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: 9b2a74a3-af59-4b1a-94ba-f05776c578e8
📒 Files selected for processing (132)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.module.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/interfaces/index.tsbackend/apps/cloud/src/analytics/protection/analytics-read.util.tsbackend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/cloud/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/ioredis.stub.tsbackend/apps/cloud/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/src/analytics/v2/dto/seo.dto.tsbackend/apps/cloud/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/dimensions.tsbackend/apps/cloud/src/analytics/v2/registry/index.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/analytics/v2/registry/seo.tsbackend/apps/cloud/src/analytics/v2/registry/types.tsbackend/apps/cloud/src/analytics/v2/seo-v2.service.tsbackend/apps/cloud/src/common/constants.tsbackend/apps/cloud/src/experiment/experiment.controller.tsbackend/apps/cloud/src/goal/goal.controller.tsbackend/apps/cloud/src/project/dto/create-project-view.dto.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.module.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/interfaces/index.tsbackend/apps/community/src/analytics/protection/analytics-read.guard.tsbackend/apps/community/src/analytics/protection/analytics-read.util.tsbackend/apps/community/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/community/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/community/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/community/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/community/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/community/src/analytics/v2/analytics-v2.service.tsbackend/apps/community/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/analytics/v2/dto/project.dto.tsbackend/apps/community/src/analytics/v2/dto/seo.dto.tsbackend/apps/community/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/community/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/community/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/community/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/community/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/community/src/analytics/v2/query/filters.translator.tsbackend/apps/community/src/analytics/v2/registry/dimensions.tsbackend/apps/community/src/analytics/v2/registry/index.tsbackend/apps/community/src/analytics/v2/registry/metrics.tsbackend/apps/community/src/analytics/v2/registry/seo.tsbackend/apps/community/src/analytics/v2/registry/types.tsbackend/apps/community/src/analytics/v2/seo-v2.service.tsbackend/apps/community/src/common/constants.tsbackend/apps/community/src/experiment/experiment.controller.tsbackend/apps/community/src/goal/goal.controller.tsbackend/apps/community/src/project/dto/create-project-view.dto.tsbackend/apps/community/src/project/gsc.service.tsbackend/apps/community/src/project/project.service.tsdocs/content/docs/api/stats-v2.mdxdocs/content/docs/api/stats.mdxweb/app/api/api.server.tsweb/app/api/v2/client.tsweb/app/api/v2/endpoints.tsweb/app/api/v2/types.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/hooks/useInViewOnce.tsweb/app/hooks/v2/useV2Queries.tsweb/app/lib/constants/index.tsweb/app/lib/v2Dimensions.tsweb/app/pages/Project/Settings/components/DeleteDataModal.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/AddAViewModal.tsxweb/app/pages/Project/View/components/FilterRowsEditor.tsxweb/app/pages/Project/View/components/Filters.tsxweb/app/pages/Project/View/components/ProjectViewHeaderActions.tsxweb/app/pages/Project/View/interfaces/traffic.tsweb/app/pages/Project/View/utils/projectViewSegments.tsweb/app/pages/Project/View/v2/BreakdownPanel.tsxweb/app/pages/Project/View/v2/TrafficMap.tsxweb/app/pages/Project/View/v2/adapters.tsweb/app/pages/Project/View/v2/loading.tsxweb/app/pages/Project/tabs/Captcha/CaptchaView.tsxweb/app/pages/Project/tabs/Captcha/adapters.tsweb/app/pages/Project/tabs/Errors/ErrorsMap.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Errors/types.tsweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/Funnels/FunnelsList.tsxweb/app/pages/Project/tabs/Funnels/FunnelsView.tsxweb/app/pages/Project/tabs/Goals/GoalSettingsModal.tsxweb/app/pages/Project/tabs/Goals/GoalsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceMap.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Performance/perfAdapters.tsweb/app/pages/Project/tabs/Profiles/ProfilesView.tsxweb/app/pages/Project/tabs/SEO/CompactReferralPanel.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/pages/Project/tabs/Sessions/SessionsView.tsxweb/app/pages/Project/tabs/Traffic/MetricCards.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/root.tsxweb/app/routes/api.analytics.tsweb/app/routes/api.v2.$.tsweb/app/routes/projects.$id.tsxweb/app/ui/FilterValueInput.tsxweb/app/ui/NumberFlow.tsxweb/app/utils/analyticsUrl.tsweb/public/locales/en.json
💤 Files with no reviewable changes (40)
- web/app/pages/Project/View/v2/loading.tsx
- backend/apps/cloud/src/analytics/v2/tests/ioredis.stub.ts
- backend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.ts
- backend/apps/community/src/analytics/protection/cacheable-analytics.decorator.ts
- backend/apps/community/src/analytics/v2/registry/types.ts
- web/app/pages/Project/tabs/Errors/types.ts
- web/app/pages/Project/tabs/Performance/PerformanceMap.tsx
- backend/apps/community/src/analytics/v2/mappers/entity.mapper.ts
- web/app/pages/Project/tabs/Captcha/adapters.ts
- backend/apps/community/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- backend/apps/cloud/src/analytics/v2/mappers/entity.mapper.ts
- backend/apps/cloud/src/analytics/v2/registry/types.ts
- backend/apps/cloud/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- backend/apps/community/src/analytics/v2/registry/dimensions.ts
- web/app/root.tsx
- backend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.ts
- backend/apps/cloud/src/analytics/v2/mappers/summary.mapper.ts
- web/app/pages/Project/tabs/Errors/ErrorsMap.tsx
- web/app/pages/Project/tabs/Profiles/ProfilesView.tsx
- web/app/pages/Project/tabs/Performance/perfAdapters.ts
- backend/apps/cloud/src/analytics/v2/registry/index.ts
- backend/apps/cloud/src/analytics/v2/registry/dimensions.ts
- web/app/hooks/useInViewOnce.ts
- backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts
- backend/apps/community/src/analytics/v2/query/breakdown-query.builder.ts
- backend/apps/community/src/analytics/v2/registry/index.ts
- backend/apps/community/src/analytics/v2/mappers/timeseries.mapper.ts
- backend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.ts
- backend/apps/cloud/src/analytics/v2/tests/registry.spec.ts
- backend/apps/community/src/analytics/v2/mappers/summary.mapper.ts
- backend/apps/community/src/analytics/v2/controllers/errors-v2.controller.ts
- backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts
- web/app/utils/analyticsUrl.ts
- web/app/pages/Project/tabs/Funnels/FunnelsView.tsx
- backend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.ts
- web/app/pages/Project/View/components/Filters.tsx
- backend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.ts
- web/app/pages/Project/View/v2/TrafficMap.tsx
- web/app/ui/FilterValueInput.tsx
- web/app/pages/Project/View/components/FilterRowsEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (51)
- backend/apps/cloud/src/common/constants.ts
- backend/apps/community/src/common/constants.ts
- backend/apps/community/src/analytics/analytics.module.ts
- web/app/lib/constants/index.ts
- backend/apps/community/src/analytics/v2/tests/registry.spec.ts
- web/app/routes/api.v2.$.ts
- web/app/pages/Project/View/interfaces/traffic.ts
- backend/apps/community/src/analytics/protection/public-project-cache.interceptor.ts
- web/app/pages/Project/Settings/components/DeleteDataModal.tsx
- web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx
- web/app/lib/v2Dimensions.ts
- backend/apps/cloud/src/project/project.service.ts
- backend/apps/community/src/analytics/v2/registry/metrics.ts
- backend/apps/community/src/project/dto/create-project-view.dto.ts
- backend/apps/community/src/project/project.service.ts
- backend/apps/community/src/analytics/protection/analytics-read.util.ts
- backend/apps/cloud/src/analytics/protection/analytics-read.util.ts
- backend/apps/community/src/analytics/v2/controllers/project-v2.controller.ts
- web/app/pages/Project/tabs/Sessions/SessionsView.tsx
- backend/apps/community/src/experiment/experiment.controller.ts
- backend/apps/community/src/analytics/v2/dto/project.dto.ts
- backend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.ts
- web/public/locales/en.json
- backend/apps/cloud/src/analytics/v2/tests/filters.translator.spec.ts
- backend/apps/cloud/src/analytics/v2/query/filters.translator.ts
- backend/apps/community/src/goal/goal.controller.ts
- docs/content/docs/api/stats.mdx
- web/app/pages/Project/tabs/Goals/GoalSettingsModal.tsx
- backend/apps/community/src/analytics/v2/query/filters.translator.ts
- web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx
- backend/apps/cloud/src/goal/goal.controller.ts
- backend/apps/community/src/analytics/v2/dto/entities.dto.ts
- backend/apps/community/src/analytics/v2/tests/filters.translator.spec.ts
- web/app/pages/Project/View/components/AddAViewModal.tsx
- backend/apps/community/src/analytics/protection/analytics-read.guard.ts
- backend/apps/cloud/src/analytics/v2/registry/metrics.ts
- web/app/routes/api.analytics.ts
- web/app/pages/Project/View/v2/BreakdownPanel.tsx
- backend/apps/cloud/src/analytics/v2/dto/project.dto.ts
- web/app/pages/Project/View/ViewProject.helpers.tsx
- web/app/pages/Project/View/v2/adapters.ts
- backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts
- web/app/pages/Project/tabs/Performance/PerformanceView.tsx
- web/app/pages/Project/tabs/Captcha/CaptchaView.tsx
- web/app/pages/Project/View/utils/projectViewSegments.ts
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
- web/app/pages/Project/tabs/Funnels/FunnelsList.tsx
- web/app/pages/Project/View/Panels.tsx
- web/app/pages/Project/tabs/Errors/ErrorsView.tsx
- web/app/pages/Project/tabs/Traffic/TrafficView.tsx
- web/app/hooks/v2/useV2Queries.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/apps/cloud/src/project/dto/create-project-view.dto.ts (1)
74-79: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale Swagger description.
The
@ApiPropertydescription still documents the old v1 filter shape{ column, filter, isExclusive, isContains }, butFilteris now{ dimension, operator, value, key? }. This misleads API consumers reading the generated docs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts` around lines 74 - 79, Update the description in the `@ApiProperty` decorator for the filter array to document the current Filter shape `{ dimension, operator, value, key? }` instead of the obsolete v1 fields, while preserving the existing optional array metadata.backend/apps/cloud/src/analytics/analytics.service.ts (1)
1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
getConcurrencyChartDatalacks error handling in both apps, so a query failure fails the whole traffic-chart response. Unlike sibling best-effort ClickHouse helpers in the same files (getOnlineUserCount,checkSessionExistsInClickHouse,getSessionDurationFromClickHouse), this new method has notry/catch; any failure propagates through thePromise.allingroupChartByTimeBucketand takes down the entire/analyticstraffic response instead of degrading to zeroed concurrency.
backend/apps/cloud/src/analytics/analytics.service.ts#L5865-5881: wrap the query intry/catch, returningArray(xShifted.length).fill(0)on error (matching the sibling helpers' fallback pattern).backend/apps/community/src/analytics/analytics.service.ts#L4413-4429: apply the same fix here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` at line 1, Add try/catch handling to getConcurrencyChartData in both analytics service implementations, returning a zero-filled array sized to xShifted.length when the ClickHouse query fails. Preserve the existing successful query result and ensure groupChartByTimeBucket receives the fallback instead of a propagated error.
🧹 Nitpick comments (8)
web/app/ui/NumberFlow.tsx (1)
156-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConditionally set
maximumFractionDigitsfor cleaner integer animations.When
showMsisfalse,srepresents a whole number. PassingmaximumFractionDigits: 2unconditionally might cause the component to interpolate through decimal values during the animation transition. To ensure the counter rolls purely as whole numbers when milliseconds are hidden, consider conditioning this format property.✨ Proposed optional refactor
{showS ? ( <NumberFlow className={FLOW_VALUE_CLASS} {...FLOW_TIMING} value={s} suffix='s' - format={{ maximumFractionDigits: 2 }} + format={{ maximumFractionDigits: showMs ? 2 : 0 }} willChange /> ) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/ui/NumberFlow.tsx` around lines 156 - 166, Update the seconds NumberFlow instance in the showS branch so its format conditionally sets maximumFractionDigits to 0 when showMs is false and retains 2 when milliseconds are shown, ensuring hidden-millisecond animations remain whole-number based.web/app/api/v2/client.ts (1)
3-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winExport
V2ApiErrorfor downstream status-based handling.
fetchV2throwsV2ApiError, but the class isn't exported, so consumers can'tinstanceof-check it or type-safely read.status(e.g., to special-case 404/429 in the v2 query hooks or tab views). Since this cohort explicitly introducesV2ApiErroras part of the client's public contract, exporting it costs nothing and unlocks structured error handling downstream.♻️ Proposed fix
-class V2ApiError extends Error { +export class V2ApiError extends Error { status: number🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/api/v2/client.ts` around lines 3 - 11, Export the V2ApiError class so downstream consumers can use instanceof checks and access its status property when handling errors from fetchV2. Keep the existing constructor, name, and status behavior unchanged.backend/apps/cloud/src/project/dto/create-project-view.dto.ts (1)
50-55: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd nested validation for persisted
Filteritems.
filters?: Filter[]only checks array-ness and length; nothing validates that each item'sdimension/operator/valueconform to the new v2 shape (e.g.operatorrestricted to'is'|'is_not'|'contains'|'contains_not'). Malformed filters could be persisted and later mis-handled by query-building code that assumes well-formed values.♻️ Suggested approach
+class ProjectViewFilterDto implements Filter { + `@IsString`() + dimension: string + + `@IsIn`(['is', 'is_not', 'contains', 'contains_not']) + operator: 'is' | 'is_not' | 'contains' | 'contains_not' + + // validate string | null | (string|null)[] as appropriate + value: string | null | (string | null)[] + + `@IsOptional`() + `@IsString`() + key?: string +} ... `@IsOptional`() `@IsArray`() `@ArrayMaxSize`(MAX_FILTERS_IN_VIEW) - filters?: Filter[] + `@ValidateNested`({ each: true }) + `@Type`(() => ProjectViewFilterDto) + filters?: ProjectViewFilterDto[]Also applies to: 81-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts` around lines 50 - 55, Update the Filter definition and the filters property in the containing DTO to perform nested validation for every persisted filter item. Ensure each item validates dimension as a string, operator against the four allowed literals, and value according to its string/null or array-of-string/null shape, while preserving the existing optional array and length constraints.backend/apps/cloud/src/analytics/analytics.service.ts (2)
140-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
MAX_CONCURRENCY_WINDOW_MINUTES(366 days) is very generous for a "live visitors" feature.The sweep-line query in
generateConcurrencyAggregationQuerymaterializes one row per minute of the window vianumbers(...). With this cap, a request combiningincludeConcurrency=truewith a large period (e.g.all/365d) can generate ~527K rows per request. Since this feature is scoped to the live-visitors chart, a much tighter cap (hours/days, not a year) would better match actual usage while bounding worst-case query cost. See consolidated comment for the duplicate in the community app.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 140 - 142, The MAX_CONCURRENCY_WINDOW_MINUTES cap is too large for the live-visitors concurrency query and can materialize hundreds of thousands of rows. Reduce this constant to a tight hours-or-days limit appropriate for the live-visitors chart, while leaving generateConcurrencyAggregationQuery and other period handling unchanged.
1-1: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
MAX_CONCURRENCY_WINDOW_MINUTES(366 days) is too generous for a live-visitors feature, duplicated in both apps. The sweep-line query materializes one row per minute of the requested window vianumbers(...); at this cap a single request can generate ~527K rows, with no other server-side guard limiting how large a window can be combined withincludeConcurrency=true.
backend/apps/cloud/src/analytics/analytics.service.ts#L140-142: lower the cap to match the live-visitors use case (hours/days, not a year).backend/apps/community/src/analytics/analytics.service.ts#L123-125: apply the same lowered cap here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` at line 1, Lower the MAX_CONCURRENCY_WINDOW_MINUTES cap in both analytics service implementations, backend/apps/cloud and backend/apps/community, to a hours/days-scale limit appropriate for live visitors instead of 366 days. Keep the same constant name and ensure both applications use the identical reduced value to bound includeConcurrency requests and sweep-line row generation.backend/apps/cloud/src/analytics/v2/seo-v2.service.ts (1)
267-296: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getSummarycomputes a meaningless "previous" period forperiod=all.Unlike
AnalyticsV2Service.getCaptchaSummary, which explicitly skips the previous-period computation whentimeframe.period === 'all', this always fetches a second full-length GSC window. Forperiod=allthat previous window sits well outside Search Console's ~16-month retention (per this file's own doc/comment about ~16-month retention), so it either burns an extra expensive GSC quota call for nothing or produces a misleadingchangecomparison against near-empty data.♻️ Suggested fix
const [current, previous] = await Promise.all([ this.gscService.getSummary(pid, groupFrom, groupTo, gscFilters, ctx), - this.gscService - .getSummary(pid, previousFrom, previousTo, gscFilters, ctx) - .catch(() => null), + timeframe.period === 'all' + ? Promise.resolve(null) + : this.gscService + .getSummary(pid, previousFrom, previousTo, gscFilters, ctx) + .catch(() => null), ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/v2/seo-v2.service.ts` around lines 267 - 296, Update getSummary to skip the previous-period GSC request when timeframe.period is 'all', passing null as the previous summary in that case. Preserve the existing previous-window calculation and error handling for all other periods.docs/content/docs/api/stats-v2.mdx (1)
247-252: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
concurrencyis missing from the "not computable per dimension" note.Line 250 calls out
session_duration/bounce_rateas excluded from/traffic/breakdown, butconcurrency(documented earlier as session-interval-derived and timeseries-only) isn't mentioned here at all. This lines up withAnalyticsV2Service.getBreakdownin the backend also lacking a runtime guard againstmetrics=concurrency— likely the same oversight on both sides.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/content/docs/api/stats-v2.mdx` around lines 247 - 252, Update the “not computable per dimension” note in the stats-v2 documentation to include concurrency alongside session_duration and bounce_rate, stating that it must be requested through timeseries or summary endpoints. Also add the corresponding runtime validation in AnalyticsV2Service.getBreakdown to reject concurrency requests for breakdown dimensions.web/app/pages/Project/tabs/SEO/SEOView.tsx (1)
245-249: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGate
referrerQueryonisConnectedlike the other SEO queries.
referrerQuery(used forsearchEngineEntries/aiReferralEntriesviaCompactReferralPanel) has noenabledcondition, unlike every other query in this component. Its results are only rendered in the JSX path reached after the!isConnectedearly return, so when GSC isn't connected this still fires a network request whose data is never displayed.♻️ Proposed fix
const referrerQuery = useBreakdownQuery('traffic', { dimension: 'referrer', limit: 100, sort: 'visitors:desc', + enabled: isConnected, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/pages/Project/tabs/SEO/SEOView.tsx` around lines 245 - 249, Update the referrerQuery useBreakdownQuery call in SEOView to include the same isConnected-based enabled condition used by the other SEO queries, preventing the request when the component is disconnected while preserving its current query parameters and rendering behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 5811-5882: Update getConcurrencyChartData to wrap the ClickHouse
query and result extraction in try/catch, matching the graceful-failure pattern
used by getOnlineUserCount, checkSessionExistsInClickHouse, and
getSessionDurationFromClickHouse. On any query or parsing error, log the failure
using the service’s established logging approach and return the appropriate
empty concurrency result so groupChartByTimeBucket continues successfully
without failing the entire traffic chart.
In `@backend/apps/community/src/analytics/analytics.service.ts`:
- Around line 4353-4430: Add the same error-handling behavior used by the cloud
app to getConcurrencyChartData around the ClickHouse query and result parsing,
ensuring failures are caught and handled consistently while preserving the
existing successful return through extractCountsForSessionChart.
In `@backend/apps/community/src/analytics/v2/analytics-v2.service.ts`:
- Line 78: Update the community analytics v2 getBreakdown flow and its related
handling around the referenced metric configuration so requests using
metrics=concurrency are explicitly rejected on /traffic/breakdown, matching the
guard behavior in the cloud analytics service and existing non-per-row metric
validation.
---
Outside diff comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Line 1: Add try/catch handling to getConcurrencyChartData in both analytics
service implementations, returning a zero-filled array sized to xShifted.length
when the ClickHouse query fails. Preserve the existing successful query result
and ensure groupChartByTimeBucket receives the fallback instead of a propagated
error.
In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts`:
- Around line 74-79: Update the description in the `@ApiProperty` decorator for
the filter array to document the current Filter shape `{ dimension, operator,
value, key? }` instead of the obsolete v1 fields, while preserving the existing
optional array metadata.
---
Nitpick comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 140-142: The MAX_CONCURRENCY_WINDOW_MINUTES cap is too large for
the live-visitors concurrency query and can materialize hundreds of thousands of
rows. Reduce this constant to a tight hours-or-days limit appropriate for the
live-visitors chart, while leaving generateConcurrencyAggregationQuery and other
period handling unchanged.
- Line 1: Lower the MAX_CONCURRENCY_WINDOW_MINUTES cap in both analytics service
implementations, backend/apps/cloud and backend/apps/community, to a
hours/days-scale limit appropriate for live visitors instead of 366 days. Keep
the same constant name and ensure both applications use the identical reduced
value to bound includeConcurrency requests and sweep-line row generation.
In `@backend/apps/cloud/src/analytics/v2/seo-v2.service.ts`:
- Around line 267-296: Update getSummary to skip the previous-period GSC request
when timeframe.period is 'all', passing null as the previous summary in that
case. Preserve the existing previous-window calculation and error handling for
all other periods.
In `@backend/apps/cloud/src/project/dto/create-project-view.dto.ts`:
- Around line 50-55: Update the Filter definition and the filters property in
the containing DTO to perform nested validation for every persisted filter item.
Ensure each item validates dimension as a string, operator against the four
allowed literals, and value according to its string/null or array-of-string/null
shape, while preserving the existing optional array and length constraints.
In `@docs/content/docs/api/stats-v2.mdx`:
- Around line 247-252: Update the “not computable per dimension” note in the
stats-v2 documentation to include concurrency alongside session_duration and
bounce_rate, stating that it must be requested through timeseries or summary
endpoints. Also add the corresponding runtime validation in
AnalyticsV2Service.getBreakdown to reject concurrency requests for breakdown
dimensions.
In `@web/app/api/v2/client.ts`:
- Around line 3-11: Export the V2ApiError class so downstream consumers can use
instanceof checks and access its status property when handling errors from
fetchV2. Keep the existing constructor, name, and status behavior unchanged.
In `@web/app/pages/Project/tabs/SEO/SEOView.tsx`:
- Around line 245-249: Update the referrerQuery useBreakdownQuery call in
SEOView to include the same isConnected-based enabled condition used by the
other SEO queries, preventing the request when the component is disconnected
while preserving its current query parameters and rendering behavior.
In `@web/app/ui/NumberFlow.tsx`:
- Around line 156-166: Update the seconds NumberFlow instance in the showS
branch so its format conditionally sets maximumFractionDigits to 0 when showMs
is false and retains 2 when milliseconds are shown, ensuring hidden-millisecond
animations remain whole-number based.
🪄 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: 9b2a74a3-af59-4b1a-94ba-f05776c578e8
📒 Files selected for processing (132)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.module.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/interfaces/index.tsbackend/apps/cloud/src/analytics/protection/analytics-read.util.tsbackend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/cloud/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/ioredis.stub.tsbackend/apps/cloud/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/cloud/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/cloud/src/analytics/v2/analytics-v2.service.tsbackend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/analytics/v2/dto/project.dto.tsbackend/apps/cloud/src/analytics/v2/dto/seo.dto.tsbackend/apps/cloud/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/cloud/src/analytics/v2/query/filters.translator.tsbackend/apps/cloud/src/analytics/v2/registry/dimensions.tsbackend/apps/cloud/src/analytics/v2/registry/index.tsbackend/apps/cloud/src/analytics/v2/registry/metrics.tsbackend/apps/cloud/src/analytics/v2/registry/seo.tsbackend/apps/cloud/src/analytics/v2/registry/types.tsbackend/apps/cloud/src/analytics/v2/seo-v2.service.tsbackend/apps/cloud/src/common/constants.tsbackend/apps/cloud/src/experiment/experiment.controller.tsbackend/apps/cloud/src/goal/goal.controller.tsbackend/apps/cloud/src/project/dto/create-project-view.dto.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.module.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/interfaces/index.tsbackend/apps/community/src/analytics/protection/analytics-read.guard.tsbackend/apps/community/src/analytics/protection/analytics-read.util.tsbackend/apps/community/src/analytics/protection/cacheable-analytics.decorator.tsbackend/apps/community/src/analytics/protection/public-project-cache.interceptor.tsbackend/apps/community/src/analytics/v2/__tests__/breakdown-query.builder.spec.tsbackend/apps/community/src/analytics/v2/__tests__/filters.translator.spec.tsbackend/apps/community/src/analytics/v2/__tests__/registry.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.mapper.spec.tsbackend/apps/community/src/analytics/v2/__tests__/seo.registry.spec.tsbackend/apps/community/src/analytics/v2/analytics-v2.service.tsbackend/apps/community/src/analytics/v2/controllers/errors-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/project-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/seo-v2.controller.tsbackend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/analytics/v2/dto/project.dto.tsbackend/apps/community/src/analytics/v2/dto/seo.dto.tsbackend/apps/community/src/analytics/v2/mappers/entity.mapper.tsbackend/apps/community/src/analytics/v2/mappers/seo.mapper.tsbackend/apps/community/src/analytics/v2/mappers/summary.mapper.tsbackend/apps/community/src/analytics/v2/mappers/timeseries.mapper.tsbackend/apps/community/src/analytics/v2/query/breakdown-query.builder.tsbackend/apps/community/src/analytics/v2/query/filters.translator.tsbackend/apps/community/src/analytics/v2/registry/dimensions.tsbackend/apps/community/src/analytics/v2/registry/index.tsbackend/apps/community/src/analytics/v2/registry/metrics.tsbackend/apps/community/src/analytics/v2/registry/seo.tsbackend/apps/community/src/analytics/v2/registry/types.tsbackend/apps/community/src/analytics/v2/seo-v2.service.tsbackend/apps/community/src/common/constants.tsbackend/apps/community/src/experiment/experiment.controller.tsbackend/apps/community/src/goal/goal.controller.tsbackend/apps/community/src/project/dto/create-project-view.dto.tsbackend/apps/community/src/project/gsc.service.tsbackend/apps/community/src/project/project.service.tsdocs/content/docs/api/stats-v2.mdxdocs/content/docs/api/stats.mdxweb/app/api/api.server.tsweb/app/api/v2/client.tsweb/app/api/v2/endpoints.tsweb/app/api/v2/types.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/hooks/useInViewOnce.tsweb/app/hooks/v2/useV2Queries.tsweb/app/lib/constants/index.tsweb/app/lib/v2Dimensions.tsweb/app/pages/Project/Settings/components/DeleteDataModal.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/AddAViewModal.tsxweb/app/pages/Project/View/components/FilterRowsEditor.tsxweb/app/pages/Project/View/components/Filters.tsxweb/app/pages/Project/View/components/ProjectViewHeaderActions.tsxweb/app/pages/Project/View/interfaces/traffic.tsweb/app/pages/Project/View/utils/projectViewSegments.tsweb/app/pages/Project/View/v2/BreakdownPanel.tsxweb/app/pages/Project/View/v2/TrafficMap.tsxweb/app/pages/Project/View/v2/adapters.tsweb/app/pages/Project/View/v2/loading.tsxweb/app/pages/Project/tabs/Captcha/CaptchaView.tsxweb/app/pages/Project/tabs/Captcha/adapters.tsweb/app/pages/Project/tabs/Errors/ErrorsMap.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Errors/types.tsweb/app/pages/Project/tabs/Experiments/ExperimentsView.tsxweb/app/pages/Project/tabs/Funnels/FunnelsList.tsxweb/app/pages/Project/tabs/Funnels/FunnelsView.tsxweb/app/pages/Project/tabs/Goals/GoalSettingsModal.tsxweb/app/pages/Project/tabs/Goals/GoalsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceMap.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Performance/perfAdapters.tsweb/app/pages/Project/tabs/Profiles/ProfilesView.tsxweb/app/pages/Project/tabs/SEO/CompactReferralPanel.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/pages/Project/tabs/Sessions/SessionsView.tsxweb/app/pages/Project/tabs/Traffic/MetricCards.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/root.tsxweb/app/routes/api.analytics.tsweb/app/routes/api.v2.$.tsweb/app/routes/projects.$id.tsxweb/app/ui/FilterValueInput.tsxweb/app/ui/NumberFlow.tsxweb/app/utils/analyticsUrl.tsweb/public/locales/en.json
💤 Files with no reviewable changes (40)
- web/app/pages/Project/View/v2/loading.tsx
- backend/apps/cloud/src/analytics/v2/tests/ioredis.stub.ts
- backend/apps/cloud/src/analytics/protection/cacheable-analytics.decorator.ts
- backend/apps/community/src/analytics/protection/cacheable-analytics.decorator.ts
- backend/apps/community/src/analytics/v2/registry/types.ts
- web/app/pages/Project/tabs/Errors/types.ts
- web/app/pages/Project/tabs/Performance/PerformanceMap.tsx
- backend/apps/community/src/analytics/v2/mappers/entity.mapper.ts
- web/app/pages/Project/tabs/Captcha/adapters.ts
- backend/apps/community/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- backend/apps/cloud/src/analytics/v2/mappers/entity.mapper.ts
- backend/apps/cloud/src/analytics/v2/registry/types.ts
- backend/apps/cloud/src/analytics/v2/tests/breakdown-query.builder.spec.ts
- backend/apps/community/src/analytics/v2/registry/dimensions.ts
- web/app/root.tsx
- backend/apps/cloud/src/analytics/v2/query/breakdown-query.builder.ts
- backend/apps/cloud/src/analytics/v2/mappers/summary.mapper.ts
- web/app/pages/Project/tabs/Errors/ErrorsMap.tsx
- web/app/pages/Project/tabs/Profiles/ProfilesView.tsx
- web/app/pages/Project/tabs/Performance/perfAdapters.ts
- backend/apps/cloud/src/analytics/v2/registry/index.ts
- backend/apps/cloud/src/analytics/v2/registry/dimensions.ts
- web/app/hooks/useInViewOnce.ts
- backend/apps/cloud/src/analytics/v2/mappers/timeseries.mapper.ts
- backend/apps/community/src/analytics/v2/query/breakdown-query.builder.ts
- backend/apps/community/src/analytics/v2/registry/index.ts
- backend/apps/community/src/analytics/v2/mappers/timeseries.mapper.ts
- backend/apps/cloud/src/analytics/protection/public-project-cache.interceptor.ts
- backend/apps/cloud/src/analytics/v2/tests/registry.spec.ts
- backend/apps/community/src/analytics/v2/mappers/summary.mapper.ts
- backend/apps/community/src/analytics/v2/controllers/errors-v2.controller.ts
- backend/apps/cloud/src/analytics/v2/controllers/errors-v2.controller.ts
- web/app/utils/analyticsUrl.ts
- web/app/pages/Project/tabs/Funnels/FunnelsView.tsx
- backend/apps/community/src/analytics/v2/controllers/traffic-v2.controller.ts
- web/app/pages/Project/View/components/Filters.tsx
- backend/apps/cloud/src/analytics/v2/controllers/traffic-v2.controller.ts
- web/app/pages/Project/View/v2/TrafficMap.tsx
- web/app/ui/FilterValueInput.tsx
- web/app/pages/Project/View/components/FilterRowsEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (51)
- backend/apps/cloud/src/common/constants.ts
- backend/apps/community/src/common/constants.ts
- backend/apps/community/src/analytics/analytics.module.ts
- web/app/lib/constants/index.ts
- backend/apps/community/src/analytics/v2/tests/registry.spec.ts
- web/app/routes/api.v2.$.ts
- web/app/pages/Project/View/interfaces/traffic.ts
- backend/apps/community/src/analytics/protection/public-project-cache.interceptor.ts
- web/app/pages/Project/Settings/components/DeleteDataModal.tsx
- web/app/pages/Project/tabs/Experiments/ExperimentsView.tsx
- web/app/lib/v2Dimensions.ts
- backend/apps/cloud/src/project/project.service.ts
- backend/apps/community/src/analytics/v2/registry/metrics.ts
- backend/apps/community/src/project/dto/create-project-view.dto.ts
- backend/apps/community/src/project/project.service.ts
- backend/apps/community/src/analytics/protection/analytics-read.util.ts
- backend/apps/cloud/src/analytics/protection/analytics-read.util.ts
- backend/apps/community/src/analytics/v2/controllers/project-v2.controller.ts
- web/app/pages/Project/tabs/Sessions/SessionsView.tsx
- backend/apps/community/src/experiment/experiment.controller.ts
- backend/apps/community/src/analytics/v2/dto/project.dto.ts
- backend/apps/cloud/src/analytics/v2/controllers/project-v2.controller.ts
- web/public/locales/en.json
- backend/apps/cloud/src/analytics/v2/tests/filters.translator.spec.ts
- backend/apps/cloud/src/analytics/v2/query/filters.translator.ts
- backend/apps/community/src/goal/goal.controller.ts
- docs/content/docs/api/stats.mdx
- web/app/pages/Project/tabs/Goals/GoalSettingsModal.tsx
- backend/apps/community/src/analytics/v2/query/filters.translator.ts
- web/app/pages/Project/View/components/ProjectViewHeaderActions.tsx
- backend/apps/cloud/src/goal/goal.controller.ts
- backend/apps/community/src/analytics/v2/dto/entities.dto.ts
- backend/apps/community/src/analytics/v2/tests/filters.translator.spec.ts
- web/app/pages/Project/View/components/AddAViewModal.tsx
- backend/apps/community/src/analytics/protection/analytics-read.guard.ts
- backend/apps/cloud/src/analytics/v2/registry/metrics.ts
- web/app/routes/api.analytics.ts
- web/app/pages/Project/View/v2/BreakdownPanel.tsx
- backend/apps/cloud/src/analytics/v2/dto/project.dto.ts
- web/app/pages/Project/View/ViewProject.helpers.tsx
- web/app/pages/Project/View/v2/adapters.ts
- backend/apps/cloud/src/analytics/v2/dto/entities.dto.ts
- web/app/pages/Project/tabs/Performance/PerformanceView.tsx
- web/app/pages/Project/tabs/Captcha/CaptchaView.tsx
- web/app/pages/Project/View/utils/projectViewSegments.ts
- web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx
- web/app/pages/Project/tabs/Funnels/FunnelsList.tsx
- web/app/pages/Project/View/Panels.tsx
- web/app/pages/Project/tabs/Errors/ErrorsView.tsx
- web/app/pages/Project/tabs/Traffic/TrafficView.tsx
- web/app/hooks/v2/useV2Queries.ts
🛑 Comments failed to post (3)
backend/apps/cloud/src/analytics/analytics.service.ts (1)
5811-5882: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Concurrency chart query has no error handling, unlike its sibling best-effort ClickHouse helpers.
getConcurrencyChartData(and the SQL it builds) has notry/catch. If this query fails for any reason (timeout, memory limit, transient CH error), the rejection propagates throughPromise.allingroupChartByTimeBucket(Lines 5915-5932) and fails the entire traffic chart response — even though concurrency is an optional, best-effort addition. Other best-effort ClickHouse lookups in this same file (getOnlineUserCount,checkSessionExistsInClickHouse,getSessionDurationFromClickHouse) all wrap their queries intry/catchand degrade gracefully. This one should follow the same pattern.🔒️ Suggested fix
async getConcurrencyChartData( timeBucket: TimeBucketType, paramsData: any, safeTimezone: string, xShifted: string[], ): Promise<number[]> { const query = this.generateConcurrencyAggregationQuery(timeBucket) - const { data } = await clickhouse - .query({ - query, - query_params: { ...paramsData.params, timezone: safeTimezone }, - }) - .then((resultSet) => resultSet.json<any>()) - - return this.extractCountsForSessionChart(data, xShifted, 'concurrency') + try { + const { data } = await clickhouse + .query({ + query, + query_params: { ...paramsData.params, timezone: safeTimezone }, + }) + .then((resultSet) => resultSet.json<any>()) + + return this.extractCountsForSessionChart(data, xShifted, 'concurrency') + } catch (error) { + console.error('[ERROR](getConcurrencyChartData):', error) + return Array(xShifted.length).fill(0) + } }📝 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.generateConcurrencyAggregationQuery(timeBucket: TimeBucketType): string { const timeBucketFunc = timeBucketConversion[timeBucket] const [selector, groupBy] = this.getGroupSubquery(timeBucket) return ` WITH parseDateTimeBestEffort({groupFrom:String}) AS fromTs, parseDateTimeBestEffort({groupTo:String}) AS toTs, toStartOfMinute(fromTs) AS fromMinute, toStartOfMinute(toTs) AS toMinute, session_intervals AS ( SELECT greatest(min(firstSeen), fromTs) AS sessionStart, least(max(lastSeen), toTs) AS sessionEnd FROM sessions WHERE pid = {pid:FixedString(12)} AND firstSeen <= toTs AND lastSeen >= fromTs GROUP BY psid ) SELECT ${selector}, toInt32(max(concurrency)) AS concurrency FROM ( SELECT ${timeBucketFunc}(toTimeZone(m, {timezone:String})) AS tz_created, concurrency FROM ( SELECT m, sum(minute_delta) OVER (ORDER BY m ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS concurrency FROM ( SELECT m, sum(delta) AS minute_delta FROM ( SELECT toStartOfMinute(sessionStart) AS m, toInt32(1) AS delta FROM session_intervals UNION ALL SELECT toStartOfMinute(sessionEnd) + INTERVAL 1 MINUTE AS m, toInt32(-1) AS delta FROM session_intervals UNION ALL SELECT fromMinute + toIntervalMinute(number) AS m, toInt32(0) AS delta FROM numbers(least(toUInt64(dateDiff('minute', fromMinute, toMinute)) + 1, ${MAX_CONCURRENCY_WINDOW_MINUTES})) ) GROUP BY m ) ) WHERE m BETWEEN fromMinute AND toMinute ) GROUP BY ${groupBy} ORDER BY ${groupBy} ` } async getConcurrencyChartData( timeBucket: TimeBucketType, paramsData: any, safeTimezone: string, xShifted: string[], ): Promise<number[]> { const query = this.generateConcurrencyAggregationQuery(timeBucket) try { const { data } = await clickhouse .query({ query, query_params: { ...paramsData.params, timezone: safeTimezone }, }) .then((resultSet) => resultSet.json<any>()) return this.extractCountsForSessionChart(data, xShifted, 'concurrency') } catch (error) { console.error('[ERROR](getConcurrencyChartData):', error) return Array(xShifted.length).fill(0) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 5811 - 5882, Update getConcurrencyChartData to wrap the ClickHouse query and result extraction in try/catch, matching the graceful-failure pattern used by getOnlineUserCount, checkSessionExistsInClickHouse, and getSessionDurationFromClickHouse. On any query or parsing error, log the failure using the service’s established logging approach and return the appropriate empty concurrency result so groupChartByTimeBucket continues successfully without failing the entire traffic chart.backend/apps/community/src/analytics/analytics.service.ts (1)
4353-4430: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Same missing error handling in
getConcurrencyChartDataas the cloud app — see consolidated comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/community/src/analytics/analytics.service.ts` around lines 4353 - 4430, Add the same error-handling behavior used by the cloud app to getConcurrencyChartData around the ClickHouse query and result parsing, ensuring failures are caught and handled consistently while preserving the existing successful return through extractCountsForSessionChart.backend/apps/community/src/analytics/v2/analytics-v2.service.ts (1)
78-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Same gap as the cloud service:
concurrencyisn't blocked on/traffic/breakdown.Identical to
backend/apps/cloud/src/analytics/v2/analytics-v2.service.ts—getBreakdown(lines 275-388) has no check preventingmetrics=concurrency, while every other non-per-row traffic metric (bounce_rate,session_duration, virtual-dimensionvisitors-only) is explicitly guarded elsewhere in this same file.Also applies to: 472-512
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/community/src/analytics/v2/analytics-v2.service.ts` at line 78, Update the community analytics v2 getBreakdown flow and its related handling around the referenced metric configuration so requests using metrics=concurrency are explicitly rejected on /traffic/breakdown, matching the guard behavior in the cloud analytics service and existing non-per-row metric validation.
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
2 participants
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.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
{ data, meta }responses, filtering, pagination, sorting, time buckets, and human-readable dimensions and metrics.