Skip to content

Comments

fix: analytics group-by chart rendering and error display#492

Draft
izadoesdev wants to merge 7 commits intouseautumn:devfrom
izadoesdev:iza-dev
Draft

fix: analytics group-by chart rendering and error display#492
izadoesdev wants to merge 7 commits intouseautumn:devfrom
izadoesdev:iza-dev

Conversation

@izadoesdev
Copy link
Contributor

@izadoesdev izadoesdev commented Dec 22, 2025

Summary

Fixes chart rendering when grouping analytics by properties and improves error messaging for invalid group-by selections.

Changes

  • Fixed chart rendering: Resolved issue where charts wouldn't render when grouping by properties (e.g., version). The backend returns data without a meta property, so we now build it from data keys when needed.
  • Improved error display: When grouping by properties with too many distinct values (>30), the UI now shows a clear error message instead of the generic "No events found" message.

Technical Details

  • Updated transformGroupedData and generateChartConfig to handle missing meta property by building it from the first data row
  • Added error message extraction and display in AnalyticsView for InvalidInputs errors related to group-by validation
  • Fixed merge conflicts between groupBy and refreshAnalytics features

Testing

  • Confirmed error message displays when attempting to group by high-cardinality properties
  • Tested with various group-by selections and data structures

Greptile Summary

  • Fixes analytics chart rendering when grouping by properties by making the meta property optional and building it from data keys when the backend doesn't provide it
  • Adds a new events_usage_mv materialized view in ClickHouse for improved analytics query performance with try-catch fallback to existing queries
  • Improves error handling in the analytics UI by displaying specific validation error messages instead of generic "No events found" when group-by has too many distinct values (>30)

Important Files Changed

Filename Overview
server/src/internal/analytics/AnalyticsService.ts Refactored to support materialized view queries with fallback mechanism and improved type safety
vite/src/views/customers/customer/analytics/utils/transformGroupedChartData.ts Fixed chart rendering by making meta property optional and building it from data keys when missing
vite/src/views/customers/customer/analytics/AnalyticsView.tsx Added proper error handling for validation errors and improved chart rendering conditions
server/src/external/clickhouse/ClickHouseManager.ts Added materialized view creation capability with special handling for DDL operations
vite/src/views/customers/customer/analytics/ViewUserEvents.tsx Contains debug code and placeholder content that appears unfinished

Confidence score: 3/5

  • This PR contains legitimate fixes for analytics chart rendering and error handling, but includes concerning elements that affect confidence
  • Score lowered due to: debug console.log statements left in production code (AnalyticsView.tsx), placeholder/unfinished component (ViewUserEvents.tsx with hardcoded text and debug output), and complex database query refactoring without comprehensive error handling
  • Pay close attention to ViewUserEvents.tsx which contains obvious development artifacts, and AnalyticsView.tsx which has debug logging that should be removed before production

Sequence Diagram

sequenceDiagram
    participant User
    participant AnalyticsView
    participant AnalyticsService
    participant ClickHouse
    participant ChartUtils

    User->>AnalyticsView: "Load analytics with group_by parameter"
    AnalyticsView->>AnalyticsService: "getTimeseriesEvents with groupBy"
    
    alt ClickHouse available
        AnalyticsService->>ClickHouse: "Query events with group by properties"
        ClickHouse-->>AnalyticsService: "Raw grouped event data"
    else ClickHouse disabled
        AnalyticsService-->>AnalyticsView: "ClickHouse disabled error"
    end
    
    alt Valid group-by property
        AnalyticsService-->>AnalyticsView: "Events data without meta property"
        AnalyticsView->>ChartUtils: "transformGroupedData(events, groupBy)"
        ChartUtils->>ChartUtils: "Build meta from data keys if missing"
        ChartUtils->>ChartUtils: "Pivot data by group values"
        ChartUtils-->>AnalyticsView: "Transformed chart data"
        
        AnalyticsView->>ChartUtils: "generateChartConfig(events, features, groupBy)"
        ChartUtils-->>AnalyticsView: "Chart configuration with colors"
        
        AnalyticsView->>User: "Display chart with grouped data"
    else Invalid group-by (high cardinality)
        AnalyticsService-->>AnalyticsView: "InvalidInputs error"
        AnalyticsView->>User: "Show specific error message for group-by validation"
    end
Loading

Context used:

  • Context from dashboard - CLAUDE.md (source)

@vercel
Copy link

vercel bot commented Dec 22, 2025

@izadoesdev is attempting to deploy a commit to the Autumn Team on Vercel.

A member of the Team first needs to authorize it.

@izadoesdev izadoesdev marked this pull request as draft December 22, 2025 19:13
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (5)

  1. vite/src/views/customers/customer/analytics/components/QueryTopbar.tsx, line 5 (link)

    style: Using deprecated UI component instead of v2

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  2. vite/src/views/customers/customer/analytics/AnalyticsView.tsx, line 123-130 (link)

    style: Debug console.log should be removed before production

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  3. server/src/internal/analytics/AnalyticsService.ts, line 345 (link)

    style: Empty catch block silently swallows all errors. Consider logging the error or being more specific about which errors should trigger the fallback. Should we log the materialized view failure for debugging purposes, or is the silent fallback intentional to avoid noise?

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  4. vite/src/views/customers/customer/analytics/ViewUserEvents.tsx, line 14 (link)

    style: Remove console.log statement - debug code should not be committed to production

  5. vite/src/views/customers/customer/analytics/ViewUserEvents.tsx, line 24-28 (link)

    logic: Dialog contains placeholder content unrelated to analytics - should either be implemented properly or removed if not ready. Is this component intended to be part of this analytics fix PR, or was it included by mistake?

12 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant