From 0d7938aff57a2ad6fdd91c72dd3a40d88a5a63a5 Mon Sep 17 00:00:00 2001 From: fabi Date: Tue, 30 Jun 2026 19:32:32 +0200 Subject: [PATCH] fix(feed): width-change measurement invalidation + best-effort SSE counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-commit review follow-ups for the batch-3 feed work. Frontend — VirtualFeed: the guarded setOptions keyed only on (count, scrollMargin), so a rotation (or the first-paint 0->real container width) changed colWidth / card heights without invalidating the cached measurements, leaving getTotalSize and the scrollbar stale until each row scrolled back through the window. Track appliedWidth and call virtualizer.measure() on a width delta so heights re-estimate at the new width (rendered rows re-measure immediately via their ResizeObserver). Covers list + grid and the first-paint 120px-estimate case. Backend — social.rs: the fresh like/comment count SELECT used `.await?`, so a transient DB failure would 500 the request after the like/comment had already committed. Make the count + broadcast best-effort (if let Ok { ... }); the mutation succeeds regardless and the next event / pull-to-refresh reconciles the count. Docs — FOLLOWUPS: record the review findings left as-is (grid-prepend tile reflow, filtered-grid auto-load which is pre-existing, comment-delete stale count, and the last-write-wins count ordering note). Verified: svelte-check 0 errors, cargo build clean. Co-Authored-By: Claude Opus 4.8 --- FOLLOWUPS.md | 35 +++++++++++++ backend/src/handlers/social.rs | 51 +++++++++++-------- .../src/lib/components/VirtualFeed.svelte | 20 ++++++-- 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/FOLLOWUPS.md b/FOLLOWUPS.md index 4a85582..5fcba09 100644 --- a/FOLLOWUPS.md +++ b/FOLLOWUPS.md @@ -79,6 +79,41 @@ excludes it; the SSR path is guarded by `getScrollElement()` returning null). - [frontend/src/routes/feed/+page.svelte](frontend/src/routes/feed/+page.svelte) — renders `VirtualFeed` for both views - [frontend/src/lib/components/FeedListCard.svelte](frontend/src/lib/components/FeedListCard.svelte) — `content-visibility` removed +**Known limitations (surfaced in the post-commit review, left as-is — low impact).** +- **Grid prepend reflows tiles.** Grid rows are index-keyed and pack 3 tiles each, so + an SSE `new-upload` shifts every tile by one position; `anchorTo:'start'` can only + anchor a scrolled grid reader when the prepend crosses a 3-item boundary (it adds a + *row*). List view is unaffected (one id-keyed row per upload). A fix would key rows + by the first tile's id and accept partial-row churn; not worth it for the rarer + "new upload while browsing the grid" case. +- **Filtered grid can auto-load the whole feed.** When a grid filter matches few items + the `VirtualFeed` is short, so the infinite-scroll sentinel sits in-viewport and + `loadMore()` fires until `nextCursor` is null — pulling all pages to widen the + client-side search. This is pre-existing (the old `FeedGrid` had the same shape, and + the empty-filter copy even says "scrolle weiter"), not a virtualization regression. + If undesired, gate auto-load to list view or to actual user scroll. + +## Feed — comment deletion leaves a stale live count + +**Problem.** `add_comment` now broadcasts a fresh `comment_count` so feed clients patch +the card in place, but `delete_comment` and `host_delete_comment` +([backend/src/handlers/social.rs](backend/src/handlers/social.rs), +[host.rs](backend/src/handlers/host.rs)) soft-delete without broadcasting any +count/event. So a deletion leaves the count too high on every client until a full +refetch (pull-to-refresh or an unrelated `upload-processed` merge). `toggle_like` +already broadcasts on both add and remove, so likes are fine — the gap is +comments-on-delete. Pre-existing, but the in-place-patch scheme makes it observable. + +**Fix.** Emit a `new-comment` (or a `comment-deleted`) event carrying the refreshed +`comment_count` from both delete paths, the same best-effort way `add_comment` does; +the frontend `patchCount(..., 'comment_count')` handler already consumes it. + +**Note — count ordering.** The broadcast count is read just after the (auto-committed) +mutation, not inside it, so under concurrent likes/comments on one upload the SSE +messages are last-write-wins. The frontend *replaces* (not increments) the count, so +steady state is correct and self-healing; only document this if strict per-event +ordering is ever required (then compute the count in-tx with a monotonic sequence). + ## Feed — per-image exact CLS reservation **Problem.** `FeedListCard` now reserves a default `aspect-[4/5]` box for photos so diff --git a/backend/src/handlers/social.rs b/backend/src/handlers/social.rs index 4b55706..31aa2cd 100644 --- a/backend/src/handlers/social.rs +++ b/backend/src/handlers/social.rs @@ -44,18 +44,22 @@ pub async fn toggle_like( } // Fresh count so feed clients can patch the single card in place instead of - // refetching page 1 (mirrors v_feed.like_count = COUNT(DISTINCT user_id)). - let like_count: i64 = - sqlx::query_scalar("SELECT COUNT(DISTINCT user_id) FROM \"like\" WHERE upload_id = $1") - .bind(upload_id) - .fetch_one(&state.pool) - .await?; - - // Broadcast SSE - let _ = state.sse_tx.send(crate::state::SseEvent { - event_type: "like-update".to_string(), - data: serde_json::json!({ "upload_id": upload_id, "like_count": like_count }).to_string(), - }); + // refetching page 1 (mirrors v_feed.like_count = COUNT(DISTINCT user_id)). The + // count + broadcast are a UI optimisation — the like itself is already committed, + // so a failure here must not fail the request. Swallow the error and skip the + // broadcast; the next event or a pull-to-refresh reconciles the count. + if let Ok(like_count) = sqlx::query_scalar::<_, i64>( + "SELECT COUNT(DISTINCT user_id) FROM \"like\" WHERE upload_id = $1", + ) + .bind(upload_id) + .fetch_one(&state.pool) + .await + { + let _ = state.sse_tx.send(crate::state::SseEvent { + event_type: "like-update".to_string(), + data: serde_json::json!({ "upload_id": upload_id, "like_count": like_count }).to_string(), + }); + } Ok(StatusCode::NO_CONTENT) } @@ -123,20 +127,23 @@ pub async fn add_comment( } // Fresh count so feed clients can patch the single card in place instead of - // refetching page 1 (mirrors v_feed.comment_count = COUNT(DISTINCT c.id)). - let comment_count: i64 = sqlx::query_scalar( + // refetching page 1 (mirrors v_feed.comment_count = COUNT(DISTINCT c.id); COUNT(*) + // over the same deleted_at filter is identical since comment.id is the PK). The + // count + broadcast are a UI optimisation — the comment is already committed, so a + // failure here must not fail the request. Swallow the error and skip the broadcast. + if let Ok(comment_count) = sqlx::query_scalar::<_, i64>( "SELECT COUNT(*) FROM comment WHERE upload_id = $1 AND deleted_at IS NULL", ) .bind(upload_id) .fetch_one(&state.pool) - .await?; - - // Broadcast SSE - let _ = state.sse_tx.send(crate::state::SseEvent { - event_type: "new-comment".to_string(), - data: serde_json::json!({ "upload_id": upload_id, "comment_count": comment_count }) - .to_string(), - }); + .await + { + let _ = state.sse_tx.send(crate::state::SseEvent { + event_type: "new-comment".to_string(), + data: serde_json::json!({ "upload_id": upload_id, "comment_count": comment_count }) + .to_string(), + }); + } let dto = CommentDto { id: comment.id, diff --git a/frontend/src/lib/components/VirtualFeed.svelte b/frontend/src/lib/components/VirtualFeed.svelte index d5fbf42..10cc8f7 100644 --- a/frontend/src/lib/components/VirtualFeed.svelte +++ b/frontend/src/lib/components/VirtualFeed.svelte @@ -106,23 +106,33 @@ // this effect (that would loop). let appliedCount = -1; let appliedMargin = Number.NaN; + let appliedWidth = Number.NaN; function applyOptions() { const count = rowCount; + const width = containerWidth; const margin = listEl ? listEl.getBoundingClientRect().top + window.scrollY : 0; scrollMargin = margin; // drives the template transforms - if (count === appliedCount && Math.abs(margin - appliedMargin) < 0.5) return; + // A width change (rotation, or the first 0→real clientWidth landing) changes + // every card's / tile-row's real height, so the heights cached from the old + // width are stale even when count and margin are unchanged. Invalidate the + // measurement cache so getTotalSize + positions recompute (rendered rows then + // re-measure immediately via their ResizeObserver) instead of trusting them. + const widthChanged = width > 0 && Math.abs(width - appliedWidth) > 0.5; + if (count === appliedCount && Math.abs(margin - appliedMargin) < 0.5 && !widthChanged) return; appliedCount = count; appliedMargin = margin; + appliedWidth = width; get(virtualizer).setOptions({ count, scrollMargin: margin }); + if (widthChanged) get(virtualizer).measure(); } - // Re-apply when the row count or column width changes (load-more, prepend, - // delete, filter, rotate). `colWidth`/`rowCount` are the only tracked reads, so a - // length-stable like/comment patch doesn't re-run this. + // Re-apply when the row count or container width changes (load-more, prepend, + // delete, filter, rotate). `containerWidth`/`rowCount` are the only tracked + // reads, so a length-stable like/comment patch doesn't re-run this. $effect(() => { void rowCount; - void colWidth; + void containerWidth; applyOptions(); });