Alternative Logs Table Approach

[trieloff]

Alternative to #122

[github-actions]

Preview deployment

Preview is live at: https://klickhaus.aemstatus.net/preview/pr-128/dashboard.html

Updated for commit edf03c3

[Copilot]

Pull request overview

This PR implements an alternative logs-table strategy that replaces offset-based infinite scroll with “gap rows” + keyset-style loading, adds on-demand full-row fetching for the detail modal, and introduces bidirectional chart↔logs scroll/scrubber synchronization with a sticky/collapsible chart layout.

Changes:

• Switch logs SQL from SELECT * to an explicit {{columns}} list and add new logs-at / log-detail templates.
• Introduce gap rows in the logs table to represent unloaded time ranges and load them on demand (click/observer) instead of OFFSET paging.
• Add chart sticky/collapse UI and bidirectional chart hover/scroll syncing via a Web Worker.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
sql/queries/logs.sql	Switch logs query to SELECT {{columns}}.
sql/queries/logs-more.sql	Replace OFFSET paging with cursor-style predicate (template).
sql/queries/logs-at.sql	New query to fetch a page “at/before” a timestamp for gap loading.
sql/queries/log-detail.sql	New detail query to fetch a full log row for the modal.
js/url-state.js	Centralize logs-vs-filters DOM sync and apply logs layout classes.
js/templates/logs-table.test.js	Add unit tests for rendering gap rows.
js/templates/logs-table.js	Add gap-row HTML builder and gap-label formatting helpers.
js/sql-loader.js	Register new SQL templates for preload/lookup.
js/scroll-sync.js	Main-thread scroll/scrubber sync module with worker integration.
js/scroll-sync-worker.js	Worker to debounce/sequence hover→check→fetch→scroll.
js/pagination.test.js	Update tests for cursor-based PaginationState.
js/pagination.js	Replace offset tracking with cursor tracking.
js/logs.js	Implement gap-row model, gap loading, on-demand detail fetch, scrubber sync, and chart collapse handling.
js/dashboard-init.js	Wire chart hover/leave callbacks and logs row hover callbacks; reprioritize logs vs facets loading.
js/columns.test.js	New tests for building safe column lists for SQL.
js/columns.js	Add buildLogColumnsSql() and always-needed columns list.
js/chart.js	Extract drawing helpers, add hover/leave hooks, and export scrubber positioning.
js/chart-draw.js	New module containing extracted canvas drawing primitives.
dashboard.html	Add chart collapse toggle button.
css/variables.css	Add CSS variables for sticky layout heights.
css/modals.css	Style “loading” state in log detail modal.
css/logs.css	Add gap-row styles and tweak sticky header z-index.
css/layout.css	Make header sticky in logs view (via :has(...)).
css/chart.css	Add sticky/collapsible chart styles and scrubber state styles.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

loadGap returns early when the request is no longer current (if (!isCurrent()) return;), but gap.gapLoading is already set to true and never reset in that path. The same happens on abort errors due to the return in the catch guard. Ensure gap.gapLoading (and the DOM) are restored on all exit paths (e.g., via finally, or resetting before each early return).

[Copilot]

Listening for scroll on document.body is unreliable for page scrolling (many browsers dispatch the scroll event on document/window, and body may not be the scrolling element). Since .logs-table-container doesn’t manage vertical scrolling, attach this handler to window (or to the actual scrolling container if that changes) to ensure scrubber sync runs while scrolling the page.

Suggested change

	document.body.addEventListener('scroll', handleLogsScroll);
	window.addEventListener('scroll', handleLogsScroll);

[Copilot]

This detail lookup filters only by timestamp and request.host, which may not uniquely identify a log row (multiple requests can share the same millisecond timestamp on the same host). Since the main logs query always includes sample_hash, consider filtering by sample_hash (optionally with timestamp as an extra guard) to guarantee the modal shows the exact selected entry.

Suggested change

	WHERE timestamp = toDateTime64('{{timestamp}}', 3) AND `request.host` = '{{host}}'
	WHERE timestamp = toDateTime64('{{timestamp}}', 3)
	AND `request.host` = '{{host}}'
	AND sample_hash = '{{sample_hash}}'

[Copilot]

Gap containment check treats gapStart as inclusive (targetMs <= gapStartMs), but gapStart is also the timestamp of a real row (the anchor above the gap). This can misclassify an actually-loaded timestamp as “in a gap” (e.g., when targetMs === gapStartMs), causing unnecessary fetches and incorrect isTimestampLoaded results. Make the upper bound strict (targetMs < gapStartMs) and/or adjust how gap rows are keyed in getItemTimestampMs to avoid equality with real rows.

Suggested change

	if (targetMs <= gapStartMs && targetMs >= gapEndMs) return { index: i, isGap: true };
	if (targetMs < gapStartMs && targetMs >= gapEndMs) return { index: i, isGap: true };

[Copilot]

showDetailLoading() injects a single <td> into #logDetailTable, but the detail table rendering uses two columns (and group rows set colspan="2"). Add colspan="2" to the loading cell (or render a 2-column row) so the loading state spans the full table width and doesn’t produce an invalid table structure.

Suggested change

	table.innerHTML = '<tbody><tr><td class="log-detail-loading">Loading full row data\u2026</td></tr></tbody>';
	table.innerHTML = '<tbody><tr><td colspan="2" class="log-detail-loading">Loading full row data\u2026</td></tr></tbody>';

[Copilot]

The mouseout handler fires whenever the pointer moves between cells within the same row (or to descendants), so onRowLeaveFn() will be called repeatedly and can cause scrubber flicker. Prefer mouseenter/mouseleave (or check e.relatedTarget to ensure the pointer actually left the row/container) before clearing the active state.

Suggested change

	if (!row) return;
	if (!row) return;
	const related = e.relatedTarget;
	if (related && row.contains(related)) return;

[Copilot]

fetchFullRow() looks up the detail row by timestamp + request.host, which may not uniquely identify the selected log entry. Since logs queries always include sample_hash, pass that through and query the detail row by a unique key (e.g., sample_hash, optionally alongside timestamp) to avoid returning the wrong record when there are multiple matches.

[Copilot]

In the worker 'fetch' handler, checkAndLoadGapFn?.(timestamp).then(...) can throw if checkAndLoadGapFn is null/undefined because .then is evaluated on the call result. Guard the call explicitly or wrap with Promise.resolve(checkAndLoadGapFn?.(timestamp)) before chaining to keep this path safe (especially if the worker posts messages before init completes).

[Copilot]

timestamp <= toDateTime64('{{target}}', 3) will include the boundary row at target. Since gaps are created using the timestamp of the last loaded row above, this will re-fetch that row and can introduce duplicates when the gap is replaced. Use a strict < comparison (or a composite keyset, e.g., (timestamp, sample_hash)) so paging into a gap never re-includes the anchor row.

Suggested change

	WHERE {{timeFilter}} AND timestamp <= toDateTime64('{{target}}', 3) {{hostFilter}} {{facetFilters}} {{additionalWhereClause}}
	WHERE {{timeFilter}} AND timestamp < toDateTime64('{{target}}', 3) {{hostFilter}} {{facetFilters}} {{additionalWhereClause}}