fix: address the review of the load more stack by chasers · Pull Request #3984 · Logflare/logflare

chasers · 2026-09-14T19:01:12Z

Stack 8/8 — fixes from a code review of PRs 1–7.

Window doubles: each click moved twice as far as the last one. One window per search now drives the label, the query, the cursor, and the range.
Timezone: an implied range was UTC but was read as local time. The range now uses the search timezone.
Open t: bound: a page request replaced t:>X with the last 2 hours. It now keeps X.
Future cursor: an empty "next" page moved the cursor past now. The cursor now stops at the request time.
Stale result: an old page result could change a new search. The LiveView now drops it.
Stuck spinner: a parse error left "Loading" on. The error path now clears it.
Other button: a click on it killed the running page query. It is now disabled while a page loads.
Cleanup: removes the unused sentinel row, dead clauses, a duplicate alias, and inline comments.
Not fixed: an empty first page still hides both buttons. That needs a design choice.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YBPF1knvgfghTSdxuYryGL

- Store one page window per search in `EventPagination`. The label, the query, the cursor shift and the range growth all read it. The window no longer doubles on each click. - Set the page window from a tail result too. `resume_tailing/1` resets the pagination, and the result that follows takes the tail path, so a soft pause, play and pause left the window nil and dropped the next page request. The window belongs to the search in view, not to the first page. - Build the chart range in the search timezone. An implied range is no longer UTC written as local time. - Keep a one-sided `t:` bound when a page request makes the range explicit. - Cap the cursor of an empty "next" page at the request time. - Clamp a "next" range extension at now. Each empty click used to push the range max another window into the future and re-run the aggregate. - Drop a page result or page error that no request waits for. - Clear the page spinner when a search fails to parse. - Disable the other button while a page request runs. - Remove the unused sentinel row: the extra fetched row, `fetch_limit/0` and the `has_more?` field that nothing reads. - Remove dead clauses, a duplicate alias and inline comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBPF1knvgfghTSdxuYryGL

depthfirst-app · 2026-09-17T19:54:49Z

    assign(socket, :event_pagination, update.(socket.assigns.event_pagination))
  end

+  defp current_page_request?(_socket, :initial), do: true


⚪ Severity: LOW

The result freshness check is not tied to a search/request identity: every :initial result is accepted, while page results are matched only by direction. A delayed result from a prior user-controlled search can reach apply_event_page_result after reset, rendering old events under new filters and exposing data outside the active search.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: Replace the unconditional true return for the :initial clause with a check of socket.assigns.loading. The loading flag is set to true whenever a new search begins and cleared to false once the initial result is applied. By tying the :initial guard to this flag, any delayed :initial result arriving after a new search has already reset loading will be correctly dropped, preventing stale data from a prior search from being rendered under the new filter context.

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change

defp current_page_request?(_socket, :initial), do: true

defp current_page_request?(socket, :initial), do: socket.assigns.loading

Squashes the nine-PR load more stack into one commit: #3976, #3977, #3978, #3979, #3980, #3981, #3982, #3984 and #3986. Merged as a unit so that main never carries the intermediate states that #3984 corrects. Pagination - Keep the "Load more" spinner up until the page query returns. - Bound a page request to the window in view and say so on the button. - Store one page window per search in `EventPagination`. The label, the query, the cursor shift and the range growth all read it. The window no longer doubles on each click. - Set the page window from a tail result too. A soft pause, play and pause used to leave the window nil and drop the next page request. - Write an implied timestamp range into the query on a page request. - Keep a one-sided `t:` bound when a page request makes the range explicit. - Clamp a "next" range extension at now. Each empty click used to push the range max another window into the future. - Cap the cursor of an empty "next" page at the request time. - Remove timestamp clauses that can never match. - Stop hiding the load more buttons on a short page. - Disable the other button while a page request runs. - Drop a page result or page error that no request waits for. - Clear the page spinner when a search fails to parse. - Log a page request that the LiveView drops. Scrolling - The LiveView drives every scroll. Every initial event page and every tail append pushes `scroll-to-bottom`. The server owns scroll intent. The hook owns viewport stability. Cleanup - Remove the unused sentinel row: the extra fetched row, `fetch_limit/0` and the `has_more?` field that nothing reads. - Build the chart range in the search timezone. An implied range is no longer UTC written as local time. - Remove the 750ms LiveView latency simulator in dev. Tests - Cover event pagination against a bigquery source and a postgres source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

chasers · 2026-09-17T21:13:40Z

Merged as part of the squashed stack in #3986, which landed on main as 9c9913b. The whole stack merged as one unit so that main never carried the intermediate states that #3984 corrects. This PR's commits are all included there.

chasers added this pull request to stack #3983 September 14, 2026 19:01

github-actions Bot assigned chasers Sep 14, 2026

chasers force-pushed the fix/load-more-short-page branch from ce4aecf to ed10f45 Compare September 14, 2026 19:20

chasers force-pushed the fix/load-more-review branch from 5830237 to e2a8472 Compare September 14, 2026 19:20

chasers force-pushed the fix/load-more-short-page branch from ed10f45 to 9ce8ac7 Compare September 17, 2026 19:30

chasers force-pushed the fix/load-more-review branch from e2a8472 to 3f42a01 Compare September 17, 2026 19:30

chasers force-pushed the fix/load-more-review branch from 3f42a01 to 670dd85 Compare September 17, 2026 19:50

depthfirst-app Bot reviewed Sep 17, 2026

View reviewed changes

amokan approved these changes Sep 17, 2026

View reviewed changes

chasers removed this pull request from stack #3983 September 17, 2026 21:09

chasers closed this Sep 17, 2026

Navigation Menu

fix: address the review of the load more stack - #3984

fix: address the review of the load more stack#3984
chasers wants to merge 1 commit into
fix/load-more-short-pagefrom
fix/load-more-review

chasers commented Sep 14, 2026

Uh oh!

depthfirst-app Bot Sep 17, 2026

Uh oh!

chasers commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	defp current_page_request?(_socket, :initial), do: true
	defp current_page_request?(socket, :initial), do: socket.assigns.loading

Conversation

chasers commented Sep 14, 2026

Uh oh!

depthfirst-app Bot Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

chasers commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants