Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions apps/admin-x-framework/src/providers/framework-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export interface StatsConfig {
endpointBrowser?: string;
id?: string;
token?: string;
version?: string;
local?: {
enabled?: boolean;
endpoint?: string;
Expand Down Expand Up @@ -86,9 +87,7 @@ export function FrameworkProvider({children, queryClientOptions, ...props}: Fram
return (
<SentryErrorBoundary>
<QueryClientProvider client={client}>
<FrameworkContext.Provider value={props}>
{children}
</FrameworkContext.Provider>
<FrameworkContext.Provider children={children} value={props} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the file
find . -name "framework-provider.tsx" -type f

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 224


🏁 Script executed:

# Check file size to understand how to read it
wc -l apps/admin-x-framework/src/providers/framework-provider.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 146


🏁 Script executed:

# Examine the area around line 90
sed -n '80,100p' apps/admin-x-framework/src/providers/framework-provider.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 547


🏁 Script executed:

# Look for Biome configuration in the repository
find . -name "biome.json" -o -name "biome.jsonc" -o -name ".biomeignore" 2>/dev/null | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 96


🏁 Script executed:

# Examine the Biome configuration
cat biome.json

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1858


🏁 Script executed:

# Search for any Biome-related linting rules about children
rg "children" biome.json -A 2 -B 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 66


🏁 Script executed:

# Let's check the React rules in Biome documentation or configuration more broadly
rg "a11y|react|children" biome.json -i

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 99


🏁 Script executed:

# Search for any ESLint or lint configuration that might flag this
find . -name ".eslintrc*" -o -name "eslint.config.*" -o -name "package.json" | head -10

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 478


🏁 Script executed:

# Let's check the actual imports and React version to understand context
head -30 apps/admin-x-framework/src/providers/framework-provider.tsx

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 981


🏁 Script executed:

# Search for Biome rules related to children or JSX in the codebase
rg "children" apps/admin-x-framework/src/providers/ -B 2 -A 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 8166


🏁 Script executed:

# Check the ESLint configuration for the admin-x-framework
cat apps/admin-x-framework/.eslintrc.cjs

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1066


🏁 Script executed:

# Let's also check for React-specific linting rules in Biome
# Search for any references to react/no-children-prop or similar
rg "react" apps/admin-x-framework/.eslintrc.cjs -i

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 531


🏁 Script executed:

# Check if there are any TypeScript or React type definitions that might validate this
rg "FrameworkContext.Provider" apps/admin-x-framework/src/ -B 2 -A 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 609


Use nested children instead of passing via prop.

ESLint's react/no-children-prop rule (from the react/recommended configuration) flags this pattern. Pass children as nested JSX content instead of as a prop.

🔧 Proposed fix
-                <FrameworkContext.Provider children={children} value={props} />
+                <FrameworkContext.Provider value={props}>
+                    {children}
+                </FrameworkContext.Provider>
📝 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
<FrameworkContext.Provider children={children} value={props} />
<FrameworkContext.Provider value={props}>
{children}
</FrameworkContext.Provider>
🧰 Tools
🪛 Biome (2.1.2)

[error] 90-90: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

🤖 Prompt for AI Agents
In `@apps/admin-x-framework/src/providers/framework-provider.tsx` at line 90, The
FrameworkContext.Provider is passing children via the children prop which trips
react/no-children-prop; change the JSX to render FrameworkContext.Provider with
nested children (i.e., <FrameworkContext.Provider value={props}> {children}
</FrameworkContext.Provider>) instead of using the children prop; update the
file's FrameworkContext.Provider usage to remove the children prop and place the
children between the opening and closing Provider tags so the context value
remains value={props}.

</QueryClientProvider>
</SentryErrorBoundary>
);
Expand Down
6 changes: 5 additions & 1 deletion apps/admin-x-framework/src/utils/stats-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ export const getStatEndpointUrl = (config?: StatsConfig | null, endpointName?: s
} else {
baseUrl = config.endpoint || '';
}
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;

return `${baseUrl}/v0/pipes/${finalEndpointName}.json?${params}`;
Comment on lines +17 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against undefined endpoint names when version is set.

With config.version and a missing endpointName, the URL becomes v2_undefined. Consider a simple guard.

🔧 Proposed fix
-    // Append version suffix if provided (e.g., "v2" -> "api_kpis_v2")
-    const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName;
-
-    return `${baseUrl}/v0/pipes/${finalEndpointName}.json?${params}`;
+    if (!endpointName) {
+        return '';
+    }
+
+    // Append version suffix if provided (e.g., "v2" -> "api_kpis_v2")
+    const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName;
+
+    return `${baseUrl}/v0/pipes/${finalEndpointName}.json?${params}`;
📝 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
// Append version suffix if provided (e.g., "v2" -> "api_kpis_v2")
const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName;
return `${baseUrl}/v0/pipes/${finalEndpointName}.json?${params}`;
if (!endpointName) {
return '';
}
// Append version suffix if provided (e.g., "v2" -> "api_kpis_v2")
const finalEndpointName = config.version ? `${config.version}_${endpointName}` : endpointName;
return `${baseUrl}/v0/pipes/${finalEndpointName}.json?${params}`;
🤖 Prompt for AI Agents
In `@apps/admin-x-framework/src/utils/stats-config.ts` around lines 17 - 20, When
building the endpoint string in stats-config (using config.version and
endpointName), guard against endpointName being undefined so you don't produce
names like `${config.version}_undefined`; update the logic that computes
finalEndpointName to check endpointName (the variable used in the template) and
either default it to a safe value (e.g., '') or throw/return an error—e.g.,
compute finalEndpointName from endpointName ?
`${config.version}_${endpointName}` : endpointName and ensure the returned URL
uses that sanitized finalEndpointName.

};

export const getToken = () => {
Expand Down
1 change: 0 additions & 1 deletion ghost/core/core/server/api/endpoints/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ const controller = {
'date_to',
'timezone',
'member_status',
'tb_version',
'post_type',
'post_uuid',
'pathname',
Expand Down
2 changes: 2 additions & 0 deletions ghost/core/core/server/data/tinybird/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ Sample config:
// -- optional override for site uuid
// "id": "106a623d-9792-4b63-acde-4a0c28ead3dc",
"endpoint": "https://api.tinybird.co",
// -- optional endpoint version suffix (e.g., "v2" calls api_kpis_v2 instead of api_kpis)
// "version": "v2",
// -- tinybird local configuration (optional)
"local": {
"enabled": true,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
SCHEMA >
`site_uuid` LowCardinality(String),
`session_id` String,
`pageviews` AggregateFunction(count, UInt64),
`first_pageview` AggregateFunction(min, DateTime),
`last_pageview` AggregateFunction(max, DateTime),
`source` AggregateFunction(argMin, String, DateTime),
`device` AggregateFunction(argMin, String, DateTime),
`utm_source` AggregateFunction(argMin, String, DateTime),
`utm_medium` AggregateFunction(argMin, String, DateTime),
`utm_campaign` AggregateFunction(argMin, String, DateTime),
`utm_term` AggregateFunction(argMin, String, DateTime),
`utm_content` AggregateFunction(argMin, String, DateTime)

ENGINE "AggregatingMergeTree"
ENGINE_SORTING_KEY "site_uuid, session_id"
4 changes: 2 additions & 2 deletions ghost/core/core/server/data/tinybird/endpoints/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
Ghost analytics distinguishes between two types of attributes:

#### Session-Level Attributes
These are captured from the **first hit** (earliest timestamp) in a session using `argMin(field, timestamp)` in the `mv_session_data` materialized view:
These are captured from the **first hit** (earliest timestamp) in a session using `argMinState(field, timestamp)` in the `_mv_session_data` materialized view (an `AggregatingMergeTree` table):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n "_mv_session_data_v2|_mv_session_data\b" ghost/core/core/server/data/tinybird -g '*.pipe' -g '*.datasource'

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1127


🏁 Script executed:

cat -n ghost/core/core/server/data/tinybird/endpoints/README.md | head -50

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 2406


🏁 Script executed:

find ghost/core/core/server/data/tinybird -name "*.pipe" | grep -E "(filtered_sessions|mv_session_data)" | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 349


🏁 Script executed:

head -30 ghost/core/core/server/data/tinybird/pipes/filtered_sessions.pipe

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1672


🏁 Script executed:

head -30 ghost/core/core/server/data/tinybird/pipes/filtered_sessions_v2.pipe

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1664


🏁 Script executed:

grep -A 10 "sessions_filtered_by_session_attributes" ghost/core/core/server/data/tinybird/pipes/filtered_sessions.pipe | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 474


🏁 Script executed:

grep -A 10 "sessions_filtered_by_session_attributes" ghost/core/core/server/data/tinybird/pipes/filtered_sessions_v2.pipe | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 459


🏁 Script executed:

grep -A 20 "sessions_filtered_by_session_attributes" ghost/core/core/server/data/tinybird/pipes/filtered_sessions_v2.pipe | head -30

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 914


🏁 Script executed:

cat -n ghost/core/core/server/data/tinybird/pipes/mv_session_data.pipe | head -30

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1424


🏁 Script executed:

cat -n ghost/core/core/server/data/tinybird/pipes/mv_session_data_v2.pipe | head -30

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 960


Clarify v1 vs v2 datasources and correct function name.

Lines 10 and 42 describe _mv_session_data, which is the v1 datasource. However, v2 endpoints use _mv_session_data_v2. Additionally, line 10 states argMinState(field, timestamp), but v1 actually uses argMin() (not State), while v2 uses argMinState(). Update the documentation to either specify it covers v1 only, or add a note clarifying v2 uses _mv_session_data_v2 with argMinState() functions.

🤖 Prompt for AI Agents
In `@ghost/core/core/server/data/tinybird/endpoints/README.md` at line 10, The
README incorrectly conflates v1 and v2 datasources and the ClickHouse function
names: update the text around _mv_session_data to explicitly state it refers to
the v1 datasource and uses argMin() (not argMinState()), and add a short note
that v2 endpoints use _mv_session_data_v2 and use argMinState() aggregates;
ensure both symbols _mv_session_data, _mv_session_data_v2 and functions argMin,
argMinState are mentioned so readers know which view and function apply to v1 vs
v2.


- `source` - Referring domain
- `utm_source` - UTM source parameter
Expand Down Expand Up @@ -39,7 +39,7 @@ Finds sessions where **at least one hit** matches the hit-level filter criteria
```sql
NODE sessions_filtered_by_session_attributes
```
Further filters by session-level attributes (source, utm_*) by joining with `mv_session_data`. These filters check attributes from the **first hit only**.
Further filters by session-level attributes (source, utm_*) by reading from `_mv_session_data` using `-Merge` combinators (e.g., `argMinMerge(source)`). These filters check attributes from the **first hit only**.

**Stage 3: Final Output**
```sql
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE _active_visitors_0
SQL >
%
select
uniqExact(session_id) as active_visitors
from _mv_hits
where
site_uuid = {{String(site_uuid, 'mock_site_uuid', description="Site UUID", required=True)}}
and timestamp >= (now() - interval 5 minute)
{% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}

TYPE ENDPOINT
162 changes: 162 additions & 0 deletions ghost/core/core/server/data/tinybird/endpoints/api_kpis_v2.pipe
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE timeseries
SQL >

%
{% set _single_day = defined(date_from) and day_diff(date_from, date_to) == 0 %}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "api_kpis_v2.pipe" -o -name "*.pipe" | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 224


🏁 Script executed:

git ls-files | grep -E "(tinybird|pipe)" | head -20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 1267


🏁 Script executed:

git ls-files | grep "api_kpis_v2.pipe"

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 147


🏁 Script executed:

cat -n ghost/core/core/server/data/tinybird/endpoints/api_kpis_v2.pipe | head -50

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 2196


🏁 Script executed:

cat -n ghost/core/core/server/data/tinybird/endpoints/api_kpis_v2.pipe

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 7570


🏁 Script executed:

# Search for day_diff usage in tinybird files to understand its behavior
rg "day_diff" --type-list | head -5

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 473


🏁 Script executed:

rg "day_diff" ghost/core/core/server/data/tinybird/ -A 2 -B 2

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 4660


Guard _single_day against undefined date_to.

Line 8: Since both date_from and date_to are optional parameters (required=False), calling day_diff(date_from, date_to) when date_to is undefined will fail. The condition should check that both are defined before invoking day_diff.

Suggested change
-        {% set _single_day = defined(date_from) and day_diff(date_from, date_to) == 0 %}
+        {% set _single_day = defined(date_from) and defined(date_to) and day_diff(date_from, date_to) == 0 %}

Note: The same issue appears in multiple locations in this file (lines 84, 124, 132).

📝 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
{% set _single_day = defined(date_from) and day_diff(date_from, date_to) == 0 %}
{% set _single_day = defined(date_from) and defined(date_to) and day_diff(date_from, date_to) == 0 %}
🤖 Prompt for AI Agents
In `@ghost/core/core/server/data/tinybird/endpoints/api_kpis_v2.pipe` at line 8,
The _single_day assignment calls day_diff(date_from, date_to) without ensuring
date_to is defined; update the condition that sets _single_day to first check
both date_from and date_to are defined (e.g., defined(date_from) and
defined(date_to)) before calling day_diff(date_from, date_to), and apply the
same guard fix to the other occurrences of this pattern in this file (the lines
using _single_day / day_diff).

with
{% if defined(date_from) %}
toStartOfDay(
toDate(
{{
Date(
date_from,
description="Starting day for filtering a date range",
required=False,
)
}}
)
) as start,
{% else %} toStartOfDay(timestampAdd(today(), interval -7 day)) as start,
{% end %}
{% if defined(date_to) %}
toStartOfDay(
toDate(
{{
Date(
date_to,
description="Finishing day for filtering a date range",
required=False,
)
}}
)
) as end
{% else %} toStartOfDay(today()) as end
{% end %}
{% if _single_day %}
select
arrayJoin(
arrayMap(
x -> toDateTime(toString(toDateTime(x)), {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}}),
range(
toUInt32(toDateTime(start)), toUInt32(timestampAdd(end, interval 1 day)), 3600
)
)
) as date
{% else %}
select
arrayJoin(
arrayMap(
x -> toDate(x),
range(toUInt32(start), toUInt32(timestampAdd(end, interval 1 day)), 24 * 3600)
)
) as date
{% end %}


NODE session_data
DESCRIPTION >
Read session data from AggregatingMergeTree MV using -Merge combinators

SQL >
%
SELECT
site_uuid,
session_id,
countMerge(pageviews) as pageviews,
minMerge(first_pageview) as first_pageview,
maxMerge(last_pageview) as last_pageview
FROM _mv_session_data_v2
WHERE site_uuid = {{ String(site_uuid, 'mock_site_uuid', description="Tenant ID", required=True) }}
GROUP BY site_uuid, session_id

NODE session_metrics
DESCRIPTION >
Calculate session-level metrics (visits, pageviews, bounce rate, avg session duration)

SQL >

%
select
site_uuid,
{% if defined(date_from) and day_diff(date_from, date_to) == 0 %}
toStartOfHour(toTimezone(first_pageview, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}})) as date,
{% else %}
toDate(toTimezone(first_pageview, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}})) as date,
{% end %}
sd.session_id,
pageviews,
pageviews = 1 as is_bounce,
last_pageview - first_pageview as session_sec
from session_data sd
inner join filtered_sessions_v2 fs
on fs.session_id = sd.session_id


NODE data
DESCRIPTION >
Calculate KPIs per time period

SQL >

select
a.date,
uniq(distinct s.session_id) as visits,
sum(s.pageviews) as pageviews,
truncate(avg(s.is_bounce), 2) as bounce_rate,
truncate(avg(s.session_sec), 2) as avg_session_sec
Comment on lines +104 to +109

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the file exists and examine the context
find . -name "api_kpis_v2.pipe" -type f

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 224


🌐 Web query:

ClickHouse uniq function syntax distinct parameter

💡 Result:

Short answer: uniq itself does not take a DISTINCT keyword parameter. Use uniq(x[, ...]) (or an alternative uniq* variant) to count distinct values; ClickHouse supports COUNT(DISTINCT ...) (which is implemented by a chosen uniq* function) and also aggregate-function DISTINCT combinator for other aggregates (e.g., sum(DISTINCT x)). See ClickHouse docs. [1][2][3]

Sources:

  • uniq function docs. [1]
  • Aggregate-function combinators (includes -Distinct / DISTINCT usage). [2]
  • count / COUNT(DISTINCT) behavior (maps to uniq* via count_distinct_implementation). [3]

🏁 Script executed:

# Also search for similar patterns in the codebase to understand how distinct counts are being done
rg "uniq|countDistinct" --type sql --max-count 20

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 224


uniq(distinct ...) is invalid ClickHouse/Tinybird SQL syntax.

Line 106: Replace with uniqExact(s.session_id) or COUNT(DISTINCT s.session_id). ClickHouse's uniq() function does not accept DISTINCT as a parameter; use a uniq* variant or standard COUNT(DISTINCT) syntax to count distinct session IDs.

Suggested change
-        uniq(distinct s.session_id) as visits,
+        uniqExact(s.session_id) as visits,
🤖 Prompt for AI Agents
In `@ghost/core/core/server/data/tinybird/endpoints/api_kpis_v2.pipe` around lines
104 - 109, The SELECT uses invalid ClickHouse syntax uniq(distinct
s.session_id); replace that expression in the SELECT list so visits is computed
correctly by using either uniqExact(s.session_id) or COUNT(DISTINCT
s.session_id) (keep the alias visits), i.e. update the select clause where
uniq(distinct s.session_id) appears in the api_kpis_v2.pipe to one of those
valid ClickHouse functions.

from timeseries a
inner join session_metrics s on a.date = s.date
group by a.date
order by a.date


NODE pathname_pageviews
DESCRIPTION >
Calculate pageviews for specific pathname with time granularity handling

SQL >

%
select
{% if defined(date_from) and day_diff(date_from, date_to) == 0 %}
toStartOfHour(toTimezone(timestamp, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}})) as date,
{% else %}
toDate(toTimezone(timestamp, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}})) as date,
{% end %}
count() pageviews
from timeseries a
inner join _mv_hits h on
{% if defined(date_from) and day_diff(date_from, date_to) == 0 %}
a.date = toStartOfHour(toTimezone(timestamp, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}}))
{% else %}
a.date = toDate(toTimezone(timestamp, {{String(timezone, 'Etc/UTC', description="Site timezone", required=True)}}))
{% end %}
inner join filtered_sessions_v2 fs
on fs.session_id = h.session_id
where
site_uuid = {{ String(site_uuid, 'mock_site_uuid', description="Tenant ID", required=True) }}
{% if defined(member_status) %} and member_status IN {{ Array(member_status, "'undefined', 'free', 'paid'", description="Member status to filter on", required=False) }} {% end %}
{% if defined(location) %} and location = {{ String(location, description="Location to filter on", required=False) }} {% end %}
{% if defined(pathname) %} and pathname = {{ String(pathname, description="Pathname to filter on", required=False) }} {% end %}
{% if defined(post_uuid) %} and post_uuid = {{String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
group by date
order by date


NODE finished_data
SQL >

%
select
a.date as date,
coalesce(b.visits, 0) as visits,
{% if defined(pathname) or defined(post_uuid) %}coalesce(c.pageviews, 0){% else %}coalesce(b.pageviews, 0){% end %} as pageviews,
coalesce(b.bounce_rate, 0) as bounce_rate,
coalesce(b.avg_session_sec, 0) as avg_session_sec
from timeseries a
left join data b on a.date = b.date
{% if defined(pathname) or defined(post_uuid) %}left join pathname_pageviews c on a.date = c.date{% end %}
TYPE ENDPOINT
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE _post_visitor_counts_0
SQL >
%
select
post_uuid,
uniqExact(session_id) as visits
from _mv_hits
where
site_uuid = {{String(site_uuid, 'mock_site_uuid', description="Site UUID", required=True)}}
and post_uuid IN {{ Array(post_uuids, description="Array of post UUIDs to get visitor counts for", required=True) }}
group by post_uuid
order by visits desc

TYPE ENDPOINT
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE session_data
SQL >
%
SELECT
site_uuid,
session_id,
argMinMerge(device) as device
FROM _mv_session_data_v2
WHERE site_uuid = {{ String(site_uuid, 'mock_site_uuid', description="Tenant ID", required=True) }}
GROUP BY site_uuid, session_id

NODE top_devices
SQL >
%
select
device,
count() as visits
from session_data sd
inner join filtered_sessions_v2 fs
on fs.session_id = sd.session_id
group by device
order by visits desc
limit {{ Int32(skip, 0) }},{{ Int32(limit, 50) }}
Comment on lines +18 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "api_top_devices_v2.pipe" -type f

Repository: qodo-benchmark/Ghost-combined-coderabbit

Length of output: 224


🌐 Web query:

Tinybird pagination ORDER BY stability offset limit deterministic ordering

💡 Result:

Short answer: use a stable, deterministic ORDER BY (include a unique tie‑breaker) or switch to keyset (cursor) pagination. OFFSET+LIMIT without a deterministic ORDER BY can return inconsistent pages.

Recommended options:

  • Deterministic OFFSET/LIMIT: ORDER BY your desired keys plus a unique column (e.g., id). Example:
    SELECT * FROM my_table
    ORDER BY created_at DESC, id DESC
    LIMIT 50 OFFSET 100.
    This guarantees stable ordering across pages. [1][2]
  • Better at scale: use keyset (cursor) pagination (WHERE (created_at, id) < (last_created_at, last_id)) and return an opaque next_cursor. This is more efficient and avoids OFFSET scan costs. [3]

References:

  • Tinybird docs: pagination with LIMIT/OFFSET and note to add ORDER BY for consistent pagination. [1]
  • Explanation and example of adding a modulus/secondary tie‑breaker and why tie‑breakers are needed for stable pagination. [2]
  • Best-practice recommendation for keyset pagination over offset pagination for stability and performance. [3]

🌐 Web query:

Tinybird .pipe SQL ORDER BY tie-breaker secondary key required

💡 Result:

Short answer: Yes — if you need deterministic ordering from a Tinybird .pipe (SELECT ... ORDER BY), include a unique secondary/tie-breaker key (e.g. a unique id or monotonic timestamp). Tinybird pipes run on ClickHouse semantics, and ClickHouse returns rows with identical ORDER BY values in arbitrary/non‑deterministic order unless you break ties with additional expressions. [1][2]

Recommendation: ORDER BY primary_col, tie_breaker_col (e.g. ORDER BY event_time DESC, id ASC) to guarantee stable results. [2]

Sources:

  • Tinybird Pipes / Query API docs. [1]
  • ClickHouse ORDER BY: rows with identical sort expressions are returned in arbitrary/non-deterministic order. [2]

Add tie-breaker to stabilize pagination ordering for deterministic results.

Line 25: When multiple devices have the same visit count, rows are returned in arbitrary order without a deterministic secondary sort key. This causes inconsistent pagination across pages when using OFFSET/LIMIT. Add device as a secondary ordering column to guarantee stable results.

Suggested change
-    order by visits desc
+    order by visits desc, device asc
📝 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
select
device,
count() as visits
from session_data sd
inner join filtered_sessions_v2 fs
on fs.session_id = sd.session_id
group by device
order by visits desc
limit {{ Int32(skip, 0) }},{{ Int32(limit, 50) }}
select
device,
count() as visits
from session_data sd
inner join filtered_sessions_v2 fs
on fs.session_id = sd.session_id
group by device
order by visits desc, device asc
limit {{ Int32(skip, 0) }},{{ Int32(limit, 50) }}
🤖 Prompt for AI Agents
In `@ghost/core/core/server/data/tinybird/endpoints/api_top_devices_v2.pipe`
around lines 18 - 26, The ORDER BY clause in the query (select device, count()
as visits from session_data sd inner join filtered_sessions_v2 fs on
fs.session_id = sd.session_id group by device order by visits desc limit ...)
lacks a stable secondary key, causing non-deterministic pagination when visits
tie; update the ORDER BY to include device as a tie-breaker (e.g., order by
visits desc, device asc) so rows with equal visits are deterministically ordered
for OFFSET/LIMIT pagination.


TYPE ENDPOINT
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE _top_locations_0
SQL >

%
select
location,
uniqExact(session_id) as visits
from _mv_hits h
inner join filtered_sessions_v2 fs
on fs.session_id = h.session_id
where
site_uuid = {{String(site_uuid, 'mock_site_uuid', description="Tenant ID", required=True)}}
{% if defined(member_status) %}
and member_status IN (
select arrayJoin(
{{ Array(member_status, "'undefined', 'free', 'paid'", description="Member status to filter on", required=False) }}
|| if('paid' IN {{ Array(member_status) }}, ['comped'], [])
)
)
{% end %}
{% if defined(location) %} and location = {{ String(location, description="Location to filter on", required=False) }} {% end %}
{% if defined(pathname) %} and pathname = {{ String(pathname, description="Pathname to filter on", required=False) }} {% end %}
{% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
Comment on lines +24 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing device filter - test expects it but endpoint doesn't implement it.

The test file api_top_locations_v2.yaml includes a test case for filtering by device=desktop (lines 78-86), but this endpoint doesn't have a condition to filter by device. This will cause the test to fail or return unexpected results.

Add the device filter condition:

             {% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
+            {% if defined(device) %} and device = {{ String(device, description="Device type to filter on", required=False) }} {% end %}
         group by location
📝 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
{% if defined(location) %} and location = {{ String(location, description="Location to filter on", required=False) }} {% end %}
{% if defined(pathname) %} and pathname = {{ String(pathname, description="Pathname to filter on", required=False) }} {% end %}
{% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
{% if defined(location) %} and location = {{ String(location, description="Location to filter on", required=False) }} {% end %}
{% if defined(pathname) %} and pathname = {{ String(pathname, description="Pathname to filter on", required=False) }} {% end %}
{% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
{% if defined(device) %} and device = {{ String(device, description="Device type to filter on", required=False) }} {% end %}
🤖 Prompt for AI Agents
In `@ghost/core/core/server/data/tinybird/endpoints/api_top_locations_v2.pipe`
around lines 24 - 26, The endpoint template is missing a device filter so tests
expecting device=desktop will fail; add a conditional block similar to the
existing ones for location/pathname/post_uuid: detect the template variable
"device" (e.g. {% if defined(device) %}) and add a filter clause that binds the
"device" parameter using the same String(...) pattern (matching description and
required=False) so the endpoint filters rows by device when provided.

group by location
order by visits desc
limit {{ Int32(skip, 0) }},{{ Int32(limit, 50) }}

TYPE ENDPOINT
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
TOKEN "stats_page" READ
TOKEN "axis" READ

NODE _top_pages_0
SQL >

%
select
case when post_uuid = 'undefined' then '' else post_uuid end as post_uuid,
pathname,
uniqExact(session_id) as visits
from _mv_hits h
inner join filtered_sessions_v2 fs
on fs.session_id = h.session_id
where
site_uuid = {{String(site_uuid, 'mock_site_uuid', description="Tenant ID", required=True)}}
{% if defined(member_status) %}
and member_status IN (
select arrayJoin(
{{ Array(member_status, "'undefined', 'free', 'paid'", description="Member status to filter on", required=False) }}
|| if('paid' IN {{ Array(member_status) }}, ['comped'], [])
)
)
{% end %}
{% if defined(location) %} and location = {{ String(location, description="Location to filter on", required=False) }} {% end %}
{% if defined(pathname) %} and pathname = {{ String(pathname, description="Pathname to filter on", required=False) }} {% end %}
{% if defined(post_uuid) %} and post_uuid = {{ String(post_uuid, description="Post UUID to filter on", required=False) }} {% end %}
{% if defined(post_type) %}
{% if post_type == 'post' %}
and post_type = 'post'
{% else %}
and (post_type != 'post' or post_type is null)
{% end %}
{% end %}
group by post_uuid, pathname
order by visits desc
limit {{ Int32(skip, 0) }},{{ Int32(limit, 50) }}

TYPE ENDPOINT
Loading