Skip to content
Merged
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
131 changes: 64 additions & 67 deletions assets/js/dashboard/extra/exploration.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ function fetchNextWithFunnel(
)
}

function fetchSuggestedJourney(site, dashboardState) {
function fetchInterestingFunnel(site, dashboardState) {
return api.post(
url.apiPath(site, '/exploration/interesting-funnel'),
dashboardState,
{}
{ max_steps: 2, max_candidates: 6 }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

JFTR, at least for the current implementation(s) of the exploration, whether we fetch 6 or 20 candidates does not really affect query performance that much.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

even for extreme sites?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I think so. The majority of work is done in the 2 innermost queries where ordering must happen on the whole resultset. Also, sort of counter-intuitively, the longer the funnel, the more performant the query because more of the transitions are filtered out early. Selecting from the sorted set is a tiny factor (that is unless you fetch by the thousands where the payload size/serialization and deserialization overhead might be more relevant)

)
}

Expand Down Expand Up @@ -219,55 +219,16 @@ export function FunnelExploration() {
// real funnel response arrives. Prevents from flashing "0 visitors"
// during the loading window.
const [provisionalFunnelEntries, setProvisionalFunnelEntries] = useState({})
// track in flight "Suggest a journey" request
const [isSuggestingJourney, setIsSuggestingJourney] = useState(false)
// counter to detect and discard stale suggestion responses
const suggestionRequestIdRef = useRef(0)
// Tracks the steps/direction/dashboardState values from the previous effect
// run so we can tell whether the journey changed (needs funnel) or only the
// search filter changed (next steps only, no funnel).
const prevStepsRef = useRef(steps)
const prevDirectionRef = useRef(direction)
const prevDashboardStateRef = useRef(dashboardState)

function cancelPendingSuggestion() {
suggestionRequestIdRef.current += 1
setIsSuggestingJourney(false)
}

function handleSuggestJourney() {
if (isSuggestingJourney) {
return
}

const requestId = ++suggestionRequestIdRef.current
setIsSuggestingJourney(true)

fetchSuggestedJourney(site, dashboardState)
.then((response) => {
// newer request (or an explicit cancel)
if (suggestionRequestIdRef.current !== requestId) {
return
}

if (response && response.length > 0) {
setSteps(response.map(({ step }) => step))
setFunnel(response)
}
})
.catch(() => {})
.finally(() => {
if (suggestionRequestIdRef.current === requestId) {
setIsSuggestingJourney(false)
}
})
}
const preloadFiredRef = useRef(false)
const funnelFromPreloadRef = useRef(false)

function handleSelect(columnIndex, selected) {
if (isSuggestingJourney) {
cancelPendingSuggestion()
}

// Reset the active-column filter whenever the journey changes
setActiveColumnFilter('')

Expand Down Expand Up @@ -307,10 +268,6 @@ export function FunnelExploration() {
function handleDirectionSelect(nextDirection) {
if (nextDirection === direction) return

if (isSuggestingJourney) {
cancelPendingSuggestion()
}

setDirection(nextDirection)
setSteps(steps.toReversed())
setFunnel([])
Expand All @@ -319,14 +276,13 @@ export function FunnelExploration() {
setProvisionalFunnelEntries({})
}

// Fetch next step suggestions (and funnel, if the journey changed) whenever
// the journey, direction, dashboard filters, or search term change.
// Funnel is only re-fetched when steps or direction/dashboardState change,
// search doesn't affect it.
// On first render fire the interesting-funnel preload and skip the normal
// next-with-funnel fetch. Once the preload resolves it sets steps
// and funnel, which re-triggers this effect for the next-step candidates fetch.
//
// On subsequent renders (via user interaction) fetch next steps and,
// if the journey changed, also refetch the funnel.
useEffect(() => {
setActiveColumnLoading(true)
setActiveColumnResults([])

const journeyChanged =
prevStepsRef.current !== steps ||
prevDirectionRef.current !== direction ||
Expand All @@ -336,14 +292,65 @@ export function FunnelExploration() {
prevDirectionRef.current = direction
prevDashboardStateRef.current = dashboardState

const includeFunnel = journeyChanged && steps.length > 0
let cancelled = false

if (!preloadFiredRef.current) {
Comment thread
zoldar marked this conversation as resolved.
preloadFiredRef.current = true
setActiveColumnLoading(true)

fetchInterestingFunnel(site, dashboardState)
.then((response) => {
if (cancelled) return
if (response && response.length > 0) {
funnelFromPreloadRef.current = true
setSteps(response.map(({ step }) => step))
setFunnel(response)
} else {
// Nothing to preload, fall back to a plain next-steps fetch
fetchNextWithFunnel(site, dashboardState, [], '', direction, false)
.then((r) => {
if (!cancelled) setActiveColumnResults(r?.next || [])
})
.catch(() => {
if (!cancelled) setActiveColumnResults([])
})
.finally(() => {
if (!cancelled) setActiveColumnLoading(false)
})
}
})
.catch(() => {
if (cancelled) return
fetchNextWithFunnel(site, dashboardState, [], '', direction, false)
.then((r) => {
if (!cancelled) setActiveColumnResults(r?.next || [])
})
.catch(() => {
if (!cancelled) setActiveColumnResults([])
})
.finally(() => {
if (!cancelled) setActiveColumnLoading(false)
})
})

return () => {
cancelled = true
}
}

setActiveColumnLoading(true)
setActiveColumnResults([])

const funnelAlreadyLoaded = funnelFromPreloadRef.current
funnelFromPreloadRef.current = false

const includeFunnel =
journeyChanged && steps.length > 0 && !funnelAlreadyLoaded

if (journeyChanged && steps.length === 0) {
setFunnel([])
}

let cancelled = false

fetchNextWithFunnel(
site,
dashboardState,
Expand Down Expand Up @@ -393,16 +400,6 @@ export function FunnelExploration() {
</h4>
</div>
<div className="flex shrink-0 items-center gap-3">
{steps.length === 0 &&
direction === EXPLORATION_DIRECTIONS.FORWARD && (
<button
onClick={handleSuggestJourney}
disabled={isSuggestingJourney}
className="text-xs font-medium text-indigo-500 hover:text-indigo-700 dark:text-indigo-400 dark:hover:text-indigo-200 disabled:opacity-50"
>
{isSuggestingJourney ? 'Suggesting...' : 'Suggest a journey'}
</button>
)}
<div className="flex gap-1 overflow-hidden">
<button
onClick={() =>
Expand Down
47 changes: 33 additions & 14 deletions extra/lib/plausible/stats/exploration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,18 @@ defmodule Plausible.Stats.Exploration do
conversion_rate_step: String.t()
}

@spec next_steps(Query.t(), journey(), String.t(), direction()) ::
{:ok, [next_step()]}
def next_steps(query, journey, search_term \\ "", direction \\ :forward)
when is_direction(direction) do
@next_steps_defaults [search_term: "", direction: :forward, max_candidates: 10]

@spec next_steps(Query.t(), journey(), keyword()) :: {:ok, [next_step()]}
def next_steps(query, journey, opts \\ []) do
opts = Keyword.merge(@next_steps_defaults, opts)
direction = Keyword.fetch!(opts, :direction)
search_term = Keyword.fetch!(opts, :search_term)
max_candidates = min(Keyword.fetch!(opts, :max_candidates), 20)

query
|> Base.base_event_query()
|> next_steps_query(journey, search_term, direction)
|> next_steps_query(journey, search_term, direction, max_candidates)
# We pass the query struct to record query metadata for
# the CH debug console.
|> ClickhouseRepo.all(query: query)
Expand Down Expand Up @@ -95,31 +100,44 @@ defmodule Plausible.Stats.Exploration do
in the journey yet. Trailing slashes are ignored when
comparing pathnames (e.g. `/foo` and `/foo/` are treated as
the same page - we should probably do that when deduplicating step candidates too).

## Options

* `:max_steps` - maximum number of funnel steps to build (default: `6`)
* `:max_candidates` - passed to `next_steps/3`, limiting
how many candidate next steps are fetched per step (default: `10`)
"""
@spec interesting_funnel(Query.t(), pos_integer()) ::
@spec interesting_funnel(Query.t(), keyword()) ::
{:ok, [funnel_step()]} | {:error, :not_found}
def interesting_funnel(query, max_steps \\ 6) do
case build_interesting_journey(query, [], MapSet.new(), max_steps) do
def interesting_funnel(query, opts \\ []) do
max_steps = min(Keyword.get(opts, :max_steps, 6), 20)
max_candidates = min(Keyword.get(opts, :max_candidates, 10), 20)

case build_interesting_journey(query, max_steps, max_candidates) do
[] -> {:error, :not_found}
journey -> journey_funnel(query, journey)
end
end

defp build_interesting_journey(_query, journey, _seen, max_steps)
defp build_interesting_journey(query, max_steps, max_candidates) do
do_build_journey(query, [], MapSet.new(), max_steps, max_candidates)
end

defp do_build_journey(_query, journey, _seen, max_steps, _max_candidates)
when length(journey) >= max_steps do
journey
end

defp build_interesting_journey(query, journey, seen, max_steps) do
{:ok, candidates} = next_steps(query, journey, "")
defp do_build_journey(query, journey, seen, max_steps, max_candidates) do
{:ok, candidates} = next_steps(query, journey, max_candidates: max_candidates)

case find_unseen_step(candidates, seen) do
nil ->
journey

step ->
new_seen = MapSet.put(seen, normalize_step_key(step))
build_interesting_journey(query, journey ++ [step], new_seen, max_steps)
do_build_journey(query, journey ++ [step], new_seen, max_steps, max_candidates)
end
end

Expand All @@ -136,7 +154,8 @@ defmodule Plausible.Stats.Exploration do
defp normalize_pathname("/"), do: "/"
defp normalize_pathname(pathname), do: String.trim_trailing(pathname, "/")

defp next_steps_query(query, steps, search_term, direction) do
defp next_steps_query(query, steps, search_term, direction, max_candidates)
when is_direction(direction) do
next_step_idx = length(steps) + 1
q_steps = steps_query(query, next_step_idx, direction)

Expand Down Expand Up @@ -171,7 +190,7 @@ defmodule Plausible.Stats.Exploration do
asc: selected_as(:next_pathname),
asc: selected_as(:next_name)
],
limit: 10
limit: ^max_candidates
)
|> maybe_search(search_term)

Expand Down
15 changes: 12 additions & 3 deletions lib/plausible_web/controllers/api/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,10 @@ defmodule PlausibleWeb.Api.StatsController do
{:ok, direction} <- parse_exploration_direction(params["direction"]),
query = Query.from(site, params, debug_metadata: debug_metadata(conn)),
{:ok, next_steps} <-
Plausible.Stats.Exploration.next_steps(query, journey, search_term, direction) do
Plausible.Stats.Exploration.next_steps(query, journey,
search_term: search_term,
direction: direction
) do
json(conn, next_steps)
else
_ ->
Expand Down Expand Up @@ -178,7 +181,10 @@ defmodule PlausibleWeb.Api.StatsController do
site = conn.assigns.site
query = Query.from(site, params, debug_metadata: debug_metadata(conn))

case Plausible.Stats.Exploration.interesting_funnel(query) do
case Plausible.Stats.Exploration.interesting_funnel(query,
max_steps: params["max_steps"],
max_candidates: params["max_candidates"]
) do
{:ok, funnel} -> json(conn, funnel)
{:error, :not_found} -> json(conn, [])
end
Expand All @@ -193,7 +199,10 @@ defmodule PlausibleWeb.Api.StatsController do
{:ok, direction} <- parse_exploration_direction(params["direction"]),
query = Query.from(site, params, debug_metadata: debug_metadata(conn)),
{:ok, next_steps} <-
Plausible.Stats.Exploration.next_steps(query, journey, search_term, direction),
Plausible.Stats.Exploration.next_steps(query, journey,
search_term: search_term,
direction: direction
),
funnel <- maybe_include_funnel(include_funnel?, query, journey, direction) do
json(conn, %{next: next_steps, funnel: funnel})
else
Expand Down
Loading
Loading