From 692e932ddf30dcdb32b61d8609dd3fb8b826f44d Mon Sep 17 00:00:00 2001 From: Chase Granberry Date: Mon, 14 Sep 2026 11:37:29 -0700 Subject: [PATCH] fix: write an implied timestamp range into the query on a page request A query with no `t:` filter now gets one on the first page request. The range is the one the chart already draws. The aggregate query charts `default_period_tick_count/1` periods back from now, so `c:group_by(t::minute)` means the last 120 minutes. The page window also uses that implied range, not a fixed 60 seconds. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YBPF1knvgfghTSdxuYryGL --- lib/logflare/logs/search_operations.ex | 32 ++++++++++++++ .../live/search_live/logs_search_lv.ex | 42 ++++++++++++++++--- .../live/search_live/logs_search_lv_test.exs | 28 +++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) diff --git a/lib/logflare/logs/search_operations.ex b/lib/logflare/logs/search_operations.ex index 8845107705..f04efcfbcc 100644 --- a/lib/logflare/logs/search_operations.ex +++ b/lib/logflare/logs/search_operations.ex @@ -474,6 +474,38 @@ defmodule Logflare.Logs.SearchOperations do %{so | query: apply_event_page_window(so.query, so, min_us, max_us)} end + @doc """ + The range a query covers when it carries no `t:` filter. + + With no filter the aggregate query charts the last `default_period_tick_count/1` periods + (`apply_bq_aggregate_timestamp_filters/4`), so that span is what the user is looking at + and what a page request should write into the query when it makes the range explicit. + """ + @spec implied_timestamp_range(atom(), DateTime.t()) :: %{ + min: NaiveDateTime.t(), + max: NaiveDateTime.t() + } + def implied_timestamp_range(chart_period, now \\ DateTime.utc_now()) + + def implied_timestamp_range(chart_period, now) + when chart_period in [:second, :minute, :hour, :day] do + seconds = + SearchOperationHelpers.default_period_tick_count(chart_period) * + period_seconds(chart_period) + + max = now |> DateTime.to_naive() |> NaiveDateTime.truncate(:second) + + %{min: NaiveDateTime.add(max, -seconds, :second), max: max} + end + + def implied_timestamp_range(_chart_period, now), + do: implied_timestamp_range(:minute, now) + + defp period_seconds(:second), do: 1 + defp period_seconds(:minute), do: 60 + defp period_seconds(:hour), do: 3_600 + defp period_seconds(:day), do: 86_400 + @doc """ Seconds a single page request may scan. diff --git a/lib/logflare_web/live/search_live/logs_search_lv.ex b/lib/logflare_web/live/search_live/logs_search_lv.ex index f0506de7d5..e4171016d7 100644 --- a/lib/logflare_web/live/search_live/logs_search_lv.ex +++ b/lib/logflare_web/live/search_live/logs_search_lv.ex @@ -852,9 +852,22 @@ defmodule LogflareWeb.Source.SearchLV do # name it. @spec page_window_seconds([term()]) :: pos_integer() defp page_window_seconds(lql_rules) do + %{min: min, max: max} = effective_or_implied_range(lql_rules) + SearchOperations.event_page_window_seconds(min, max) + end + + # A query without a `t:` filter still covers a window: the one the chart is drawing, which + # the aggregate query derives from the chart period. That is what the user is looking at, + # so that is what a page request measures itself against. + defp effective_or_implied_range(lql_rules) do case Rules.effective_timestamp_range(lql_rules) do - %{min: min, max: max} -> SearchOperations.event_page_window_seconds(min, max) - _ -> 60 + %{min: _, max: _} = range -> + range + + _ -> + lql_rules + |> Rules.get_chart_period(:minute) + |> SearchOperations.implied_timestamp_range() end end @@ -950,12 +963,14 @@ defmodule LogflareWeb.Source.SearchLV do # whether or not the window held any events. Extending only as far as the rows that came # back would stall the moment a page landed on a quiet stretch. defp advance_page(socket, event_page, intent) do + # The window, and the range it is written into, both describe what was on screen when + # the button was clicked, which is what its label promised. + cursors = socket.assigns.pagination_cursors window = page_window_seconds(socket.assigns.lql_rules) - previous_cursor = Map.get(socket.assigns.pagination_cursors, intent) socket |> put_event_page_result(event_page, intent) - |> keep_cursor_moving(event_page, intent, previous_cursor, window) + |> keep_cursor_moving(event_page, intent, Map.get(cursors, intent), window) |> extend_timestamp_range_by(intent, window) end @@ -969,6 +984,20 @@ defmodule LogflareWeb.Source.SearchLV do defp keep_cursor_moving(socket, _event_page, _intent, _previous_cursor, _window), do: socket + # Paging from a query with no `t:` filter writes the chart's own window into the query, so + # the range in the URL always describes what is on screen. + defp make_timestamp_range_explicit(lql_rules) do + case Rules.effective_timestamp_range(lql_rules) do + %{min: _, max: _} -> + lql_rules + + _ -> + %{min: min, max: max} = effective_or_implied_range(lql_rules) + rule = FilterRule.build(path: "timestamp", operator: :range, values: [min, max]) + Rules.update_timestamp_rules(lql_rules, [rule]) + end + end + defp shift_cursor(nil, _intent, _window), do: nil defp shift_cursor(%{timestamp: timestamp} = cursor, :previous, window), @@ -978,7 +1007,10 @@ defmodule LogflareWeb.Source.SearchLV do do: %{cursor | timestamp: timestamp + window * 1_000_000} defp extend_timestamp_range_by(socket, intent, window) do - lql_rules = adjust_timestamp_rules(socket.assigns.lql_rules, socket.assigns.search_timezone) + lql_rules = + socket.assigns.lql_rules + |> adjust_timestamp_rules(socket.assigns.search_timezone) + |> make_timestamp_range_explicit() case Rules.effective_timestamp_range(lql_rules) do %{min: min, max: max} -> diff --git a/test/logflare_web/live/search_live/logs_search_lv_test.exs b/test/logflare_web/live/search_live/logs_search_lv_test.exs index d2c8452afa..bd5be570e8 100644 --- a/test/logflare_web/live/search_live/logs_search_lv_test.exs +++ b/test/logflare_web/live/search_live/logs_search_lv_test.exs @@ -2442,6 +2442,34 @@ defmodule LogflareWeb.Source.SearchLVTest do assert_push_event(view, "scroll-to-event", %{id: ^oldest_loaded_id}) end + test "paging from an implied range writes an explicit one into the query", %{ + conn: conn, + events: events, + message_prefix: message_prefix, + source: source + } do + querystring = "#{message_prefix} c:count(*) c:group_by(t::minute)" + + view = open_pagination_search(conn, source, querystring, Enum.at(events, 102)) + + refute Rules.effective_timestamp_range(get_view_assigns(view).lql_rules) + assert has_element?(view, "#load-more-events-top:not([disabled])") + + assert_push_event(view, "scroll-to-bottom", %{}, 5_000) + + view + |> element("#load-more-events-top") + |> render_click() + + assert_patch(view) + + TestUtils.retry_assert(fn -> + assigns = get_view_assigns(view) + assert %{min: _, max: _} = Rules.effective_timestamp_range(assigns.lql_rules) + assert assigns.querystring =~ "t:20" + end) + end + test "the top button shows for a single-page range and loads older events from outside it", %{conn: conn, events: events, message_prefix: message_prefix, source: source} do range_start = div(Enum.at(events, 3).body["timestamp"], 1_000_000) - 1