Added materialized view and duplicated v2 Tinybird endpoints#26
Conversation
…re robust polling approach
There was a problem hiding this comment.
Pull request overview
This PR introduces materialized view optimizations for Tinybird analytics by creating v2 versions of all analytics endpoints. The changes refactor the versioning approach from a runtime parameter (tbVersion) to a configuration-based system where version suffixes are added to endpoint names.
Changes:
- Adds
_mv_session_data_v2materialized view usingAggregatingMergeTreefor optimized session data storage - Creates v2 versions of 12 analytics endpoints with
_v2suffix (e.g.,api_kpis_v2,api_top_pages_v2) - Refactors versioning from runtime
tbVersionparameter to config-basedversionfield - Removes
tb_versionandsourceparameters from API and service layers - Adds
filtered_sessions_v2pipe for v2 endpoint filtering logic
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ghost/core/core/server/services/tinybird/TinybirdService.js | Adds v2 pipe names to the TINYBIRD_PIPES array |
| ghost/core/core/server/services/stats/utils/tinybird.js | Refactors URL construction to use config-based version suffix |
| ghost/core/core/server/services/stats/ContentStatsService.js | Removes tb_version and source parameter handling |
| ghost/core/core/server/api/endpoints/stats.js | Removes tb_version from accepted options |
| apps/admin-x-framework/src/utils/stats-config.ts | Adds version suffix to endpoint URLs (contains critical bug) |
| apps/admin-x-framework/src/providers/framework-provider.tsx | Adds version field to StatsConfig interface and minor JSX style change |
| ghost/core/core/server/data/tinybird/datasources/_mv_session_data_v2.datasource | Defines schema for v2 materialized view datasource |
| ghost/core/core/server/data/tinybird/pipes/mv_session_data_v2.pipe | Implements v2 materialized view with aggregate functions |
| ghost/core/core/server/data/tinybird/pipes/filtered_sessions_v2.pipe | Implements v2 session filtering logic |
| ghost/core/core/server/data/tinybird/endpoints/api_*_v2.pipe | Creates 12 v2 endpoint implementations |
| ghost/core/core/server/data/tinybird/tests/api_*_v2.yaml | Adds comprehensive test fixtures for all v2 endpoints |
| ghost/core/core/server/data/tinybird/README.md | Documents the new version configuration option |
| ghost/core/core/server/data/tinybird/endpoints/README.md | Updates documentation to reflect v2 materialized view approach |
| ghost/core/test/unit/server/services/stats/utils/tinybird.test.js | Updates tests for config-based versioning |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return `${baseUrl}/v0/pipes/${endpointName}.json?${params}`; | ||
|
|
||
| // Append version suffix if provided (e.g., "v2" -> "api_kpis_v2") | ||
| const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName; |
There was a problem hiding this comment.
The version suffix pattern is inconsistent between backend and frontend code. The backend applies the version as pipeName_version (e.g., api_kpis_v2), while the frontend applies it as version_pipeName (e.g., v2_api_kpis). This will result in incorrect endpoint URLs being generated from the frontend, causing API calls to fail.
The backend pattern in tinybird.js line 38-40:
const pipeUrl = version ?
`/v0/pipes/${pipeName}_${version}.json` :
`/v0/pipes/${pipeName}.json`;The frontend pattern in stats-config.ts line 18:
const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName;These should match. Based on the test expectations and the actual v2 pipe names (e.g., api_kpis_v2), the frontend implementation needs to be corrected to use endpointName_version instead of version_endpointName.
| const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName; | |
| const finalEndpointName = config.version ? `${endpointName}_${config.version}` : endpointName; |
| <FrameworkContext.Provider value={props}> | ||
| {children} | ||
| </FrameworkContext.Provider> | ||
| <FrameworkContext.Provider children={children} value={props} /> |
There was a problem hiding this comment.
This change from the explicit JSX children pattern to using the children prop shorthand is purely a stylistic change. While both patterns are functionally equivalent in React, this change appears unrelated to the PR's stated purpose of adding materialized views and v2 endpoints. Consider removing this change or documenting why it's included in this PR.
| <FrameworkContext.Provider children={children} value={props} /> | |
| <FrameworkContext.Provider value={props}> | |
| {children} | |
| </FrameworkContext.Provider> |
| tinybirdOptions.location = options.location; | ||
| } | ||
|
|
||
| // Only add source if defined (allow empty string for "Direct" traffic) | ||
| if (options.source !== undefined) { | ||
| tinybirdOptions.source = options.source; | ||
| } | ||
|
|
||
| // Only add UTM parameters if they are defined (not undefined/null) | ||
| if (options.utm_source) { | ||
| tinybirdOptions.utmSource = options.utm_source; |
There was a problem hiding this comment.
The code that handled the source parameter has been removed, but the JSDoc documentation at line 37 still lists options.source as a valid parameter. This creates inconsistency between the documentation and implementation. Either restore the source parameter handling or update the JSDoc to remove the reference to this parameter.
Benchmark PR from qodo-benchmark#232