WIP: Pilot safe OpenCode pull request reviews

lambadalambda commented

2026-08-03 08:27:02 +00:00

Owner

Summary

track the isolated OpenCode review bot pilot
use this pull request for the first live requestable review

Testing

pending bot deployment and live review

## Summary - track the isolated OpenCode review bot pilot - use this pull request for the first live requestable review ## Testing - pending bot deployment and live review

lambadalambda added 20 commits

2026-08-03 08:27:02 +00:00

fix: improve large thread navigation 09ef3e9f99

fix: remove thread ending seam c3fbbb6339

test: stabilize stale thread navigation 1b27e047ef

fix: render emoji in reply previews da499fc9bd

fix: reliably catch up timeline streams a8895869f0

docs: record custom emoji audit 0e5de617a0

fix: reconcile replies into open threads 5c47301425

fix: reveal thread reply composers 1287c77363

feat: add compact media previews 780a80759f

fix: render custom emoji across app surfaces ebc0e7751a

feat: expand compact image previews 1c8b63993e

feat: add fit-images timeline setting bee9b01388

fix: insert replies to thread ancestors e287f6e5d9

feat: share fit-images setting with threads 1901a5780d

fix: reconcile post counts from notifications 922ec5aff6

feat: show and inherit post visibility 651e256edd

fix: align visibility with post timestamp 00b40b67f8

feat: align notifications page with handoff 2f886e44f0

fix: use available lightbox height 87167d89dc

docs: track safe OpenCode review pilot

opencode/review OpenCode execution completed

Details

93280f3bc3

lambadalambda commented

2026-08-03 11:03:52 +00:00

Author

Owner

/oc review

opencode-bot commented

2026-08-03 11:04:06 +00:00

Collaborator

OpenCode Review

Execution completed for lambadalambda/pleromanet at 1a043729493a (job #6).

Findings

Medium

Unbounded growth of status-engagement tracking maps — src/routes/app/[...path]/+page.svelte
The new revision-tracking structures (statusEngagementSnapshots, statusEngagementNotificationRevisions, statusEngagementNotificationArrivals, notificationEngagementSignatures, seenNotificationEngagementSignatures) are keyed by status/notification IDs and never pruned. invalidateStatusActionRequests resets them only on session/route invalidation, so a long-running session that views many posts will accumulate entries indefinitely. Consider evicting entries for status IDs no longer present in any loaded surface, or limiting to the most-recent N targets. This is the most concrete memory-growth concern introduced by the count-reconciliation feature.

Floating image preview is repositioned on every scroll/resize without throttling — src/lib/rebuild/CompactMediaPreview.svelte
positionFullPhoto is bound to window.scroll (capture), window.resize, visualViewport.resize, and visualViewport.scroll, plus image onload. Each invocation performs getBoundingClientRect() reads, multiple await tick() calls, and updates fullPhotoStyle. There is no throttle, debounce, or requestAnimationFrame coalescing. On lower-end devices and long hover sessions this can introduce jank, especially because the effect tears down and re-creates listeners on every fullPhoto/fullPhotoAnchor change. Consider an rAF-batched reposition and keeping listeners attached for the lifetime of the open preview (they are already gated by the open state).

Low

Misleading time-bucket labels in NotificationsPage — src/lib/rebuild/NotificationsPage.svelte
bucketFor maps ages 1–6 h to "Today", 6–48 h to "Yesterday", and 2–7 days to "This week". The labels do not match the implied semantics and will be confusing for users (e.g., a 7-hour-old notification shows under "Yesterday"). Either rename the buckets (e.g., "Recent", "Today", "Earlier this week") or compute them against calendar days.

Redundant condition in WebSocket onerror handler — src/lib/pleroma/streaming.ts

socket.onerror = (event) => {
    if (closed) return;
    if (!closed) onError?.(event);
};

The inner if (!closed) is unreachable because closed cannot change synchronously between the two checks. Collapse to a single onError?.(event); after the early return.

role="note" is non-standard — src/lib/rebuild/CompactMediaPreview.svelte

<div class="compact-media-preview compact-media-hidden" role="note">Sensitive media</div>

note is not in the WAI-ARIA 1.2 vocabulary; some screen readers will ignore it. Use a standard role such as status, or omit role and rely on the visible text.

Notification actionable row loses pointer affordance — src/app.css, src/lib/rebuild/NotifRow.svelte
The old rule .notif-row.actionable { cursor: pointer; } was removed and the actionable surface is now a real <button class="notif-row-text">. Browser default cursor for <button> is default, so the row no longer signals clickability. Add cursor: pointer to .notif-row-text:is(button) (or rely on the existing .notif-quote-open:is(button) pattern).

Two distinct buttons open the same notification — src/lib/rebuild/NotifRow.svelte
For actionable rows with a post, both .notif-row-text (the whole row) and .notif-quote-open (the excerpt) call onOpen?.(n). This creates redundant focusable targets and a duplicated "open" action in the a11y tree. Either expose a single activator or mark the secondary one tabindex="-1"/aria-hidden.

Double notification load after clear — src/routes/app/[...path]/+page.svelte
clearNotifications first runs loadNotifications(session, { replace: true }) and then, in finally, unconditionally calls loadNotifications(session, { background: true }). The replace load already fetches authoritative state right after the clear; the second call is a duplicate network request in the common success path. Gate the background follow-up on a real signal (e.g., the replacement stream has opened with no fresh data) or skip it when the replace succeeded.

CompactMediaPreview portal lifecycle is fragile — src/lib/rebuild/CompactMediaPreview.svelte
The portal action appends node to document.body and removes it on destroy. Svelte also tries to remove the node from its (now-moved) parent during {#if fullPhoto} teardown. The current ordering works today, but it depends on Svelte calling the action destroy before attempting DOM removal. Adding a comment or guarding the destroy to only remove when still attached would future-proof this against Svelte internals changes.

aria-label on visibility span may double-read — src/lib/rebuild/PostVisibility.svelte
The outer <span> carries aria-label while wrapping an Icon plus a visible <span>{details.label}</span>. If Icon exposes its own accessible name, AT users may hear the label twice. Ensure the inner Icon is aria-hidden="true" (the component is not shown in the patch).

Informational

timeline-catch-up.ts and reconcileTimelineCatchUp are intricate but accompanied by a substantial, focused Playwright suite covering multi-page catch-up, concurrent streamed posts, empty-timeline recovery, route retention, and stale local vs. federated races. The behavior matches the tests; future maintainers will benefit from inline comments explaining the initialDataIds/initialQueuedIds overlap algorithm.
Notification clearing correctly bumps notificationStreamGeneration, gates applyStreamedNotification on both stream generation and notificationClearState === 'clearing', and re-checks isCurrentSessionRequest after each await. The in-flight-notification and delayed-stream tests exercise the important races.
statusReplyTargetId/matchesStatusReplyTarget correctly route boosted-source identity through thread reconciliation, ancestor replies, and reply-target expansion. Indexing byId by both id and statusReplyTargetId is safe under the assumption that source IDs are unique within a thread context.
compactExcerpt shortcode-boundary logic is exported and unit-tested, and is reused for chat excerpts after send — a nice consolidation of the prior chatExcerpt helper.
Custom-emoji propagation is consistently applied to the audit-identified surfaces (notifications, CW summaries, pinned posts, chat rows, mention suggestions, lightbox attribution, profile context, settings preview), and hidden-emoji/no-request assertions back the privacy claims.
openPleromaTimelineStream changes (open-timeout, detach-on-close, idempotent close) are well-covered by the rewritten client tests, including the new "socket never opens" replacement case.

Overall

A large, well-structured patch that delivers a coherent set of features (fit-images, compact media previews with floating full-image view, notifications-page redesign, visibility badges, timeline stream catch-up, thread loading/scroll/restoration, count reconciliation, custom-emoji audit fixes, clear-all with generation-safe catch-up). Test coverage is thorough and traces cleanly to the per-issue acceptance criteria. No security, data-integrity, or correctness defects were identified. The most actionable items are the unbounded engagement-snapshot maps (Medium), the unthrottled floating-preview positioning (Medium), and a handful of low-severity UX/a11y/style nits listed above.

## OpenCode Review _Execution completed for `lambadalambda/pleromanet` at `1a043729493a` (job #6)._ ## Findings ### Medium **Unbounded growth of status-engagement tracking maps** — `src/routes/app/[...path]/+page.svelte` The new revision-tracking structures (`statusEngagementSnapshots`, `statusEngagementNotificationRevisions`, `statusEngagementNotificationArrivals`, `notificationEngagementSignatures`, `seenNotificationEngagementSignatures`) are keyed by status/notification IDs and never pruned. `invalidateStatusActionRequests` resets them only on session/route invalidation, so a long-running session that views many posts will accumulate entries indefinitely. Consider evicting entries for status IDs no longer present in any loaded surface, or limiting to the most-recent N targets. This is the most concrete memory-growth concern introduced by the count-reconciliation feature. **Floating image preview is repositioned on every scroll/resize without throttling** — `src/lib/rebuild/CompactMediaPreview.svelte` `positionFullPhoto` is bound to `window.scroll` (capture), `window.resize`, `visualViewport.resize`, and `visualViewport.scroll`, plus image `onload`. Each invocation performs `getBoundingClientRect()` reads, multiple `await tick()` calls, and updates `fullPhotoStyle`. There is no throttle, debounce, or `requestAnimationFrame` coalescing. On lower-end devices and long hover sessions this can introduce jank, especially because the effect tears down and re-creates listeners on every `fullPhoto`/`fullPhotoAnchor` change. Consider an `rAF`-batched reposition and keeping listeners attached for the lifetime of the open preview (they are already gated by the open state). ### Low **Misleading time-bucket labels in NotificationsPage** — `src/lib/rebuild/NotificationsPage.svelte` `bucketFor` maps ages 1–6 h to "Today", 6–48 h to "Yesterday", and 2–7 days to "This week". The labels do not match the implied semantics and will be confusing for users (e.g., a 7-hour-old notification shows under "Yesterday"). Either rename the buckets (e.g., "Recent", "Today", "Earlier this week") or compute them against calendar days. **Redundant condition in WebSocket onerror handler** — `src/lib/pleroma/streaming.ts` ```js socket.onerror = (event) => { if (closed) return; if (!closed) onError?.(event); }; ``` The inner `if (!closed)` is unreachable because `closed` cannot change synchronously between the two checks. Collapse to a single `onError?.(event);` after the early return. **`role="note"` is non-standard** — `src/lib/rebuild/CompactMediaPreview.svelte` ```svelte <div class="compact-media-preview compact-media-hidden" role="note">Sensitive media</div> ``` `note` is not in the WAI-ARIA 1.2 vocabulary; some screen readers will ignore it. Use a standard role such as `status`, or omit `role` and rely on the visible text. **Notification actionable row loses pointer affordance** — `src/app.css`, `src/lib/rebuild/NotifRow.svelte` The old rule `.notif-row.actionable { cursor: pointer; }` was removed and the actionable surface is now a real `<button class="notif-row-text">`. Browser default cursor for `<button>` is `default`, so the row no longer signals clickability. Add `cursor: pointer` to `.notif-row-text:is(button)` (or rely on the existing `.notif-quote-open:is(button)` pattern). **Two distinct buttons open the same notification** — `src/lib/rebuild/NotifRow.svelte` For actionable rows with a post, both `.notif-row-text` (the whole row) and `.notif-quote-open` (the excerpt) call `onOpen?.(n)`. This creates redundant focusable targets and a duplicated "open" action in the a11y tree. Either expose a single activator or mark the secondary one `tabindex="-1"`/`aria-hidden`. **Double notification load after clear** — `src/routes/app/[...path]/+page.svelte` `clearNotifications` first runs `loadNotifications(session, { replace: true })` and then, in `finally`, unconditionally calls `loadNotifications(session, { background: true })`. The replace load already fetches authoritative state right after the clear; the second call is a duplicate network request in the common success path. Gate the background follow-up on a real signal (e.g., the replacement stream has opened with no fresh data) or skip it when the replace succeeded. **`CompactMediaPreview` portal lifecycle is fragile** — `src/lib/rebuild/CompactMediaPreview.svelte` The `portal` action appends `node` to `document.body` and removes it on destroy. Svelte also tries to remove the node from its (now-moved) parent during `{#if fullPhoto}` teardown. The current ordering works today, but it depends on Svelte calling the action `destroy` before attempting DOM removal. Adding a comment or guarding the destroy to only remove when still attached would future-proof this against Svelte internals changes. **`aria-label` on visibility span may double-read** — `src/lib/rebuild/PostVisibility.svelte` The outer `<span>` carries `aria-label` while wrapping an `Icon` plus a visible `<span>{details.label}</span>`. If `Icon` exposes its own accessible name, AT users may hear the label twice. Ensure the inner `Icon` is `aria-hidden="true"` (the component is not shown in the patch). ### Informational - **`timeline-catch-up.ts` and `reconcileTimelineCatchUp`** are intricate but accompanied by a substantial, focused Playwright suite covering multi-page catch-up, concurrent streamed posts, empty-timeline recovery, route retention, and stale local vs. federated races. The behavior matches the tests; future maintainers will benefit from inline comments explaining the `initialDataIds`/`initialQueuedIds` overlap algorithm. - **Notification clearing** correctly bumps `notificationStreamGeneration`, gates `applyStreamedNotification` on both stream generation and `notificationClearState === 'clearing'`, and re-checks `isCurrentSessionRequest` after each `await`. The in-flight-notification and delayed-stream tests exercise the important races. - **`statusReplyTargetId`/`matchesStatusReplyTarget`** correctly route boosted-source identity through thread reconciliation, ancestor replies, and reply-target expansion. Indexing `byId` by both `id` and `statusReplyTargetId` is safe under the assumption that source IDs are unique within a thread context. - **`compactExcerpt` shortcode-boundary logic** is exported and unit-tested, and is reused for chat excerpts after send — a nice consolidation of the prior `chatExcerpt` helper. - **Custom-emoji propagation** is consistently applied to the audit-identified surfaces (notifications, CW summaries, pinned posts, chat rows, mention suggestions, lightbox attribution, profile context, settings preview), and hidden-emoji/no-request assertions back the privacy claims. - **`openPleromaTimelineStream` changes** (open-timeout, detach-on-close, idempotent close) are well-covered by the rewritten client tests, including the new "socket never opens" replacement case. ## Overall A large, well-structured patch that delivers a coherent set of features (fit-images, compact media previews with floating full-image view, notifications-page redesign, visibility badges, timeline stream catch-up, thread loading/scroll/restoration, count reconciliation, custom-emoji audit fixes, clear-all with generation-safe catch-up). Test coverage is thorough and traces cleanly to the per-issue acceptance criteria. No security, data-integrity, or correctness defects were identified. The most actionable items are the unbounded engagement-snapshot maps (Medium), the unthrottled floating-preview positioning (Medium), and a handful of low-severity UX/a11y/style nits listed above.

lambadalambda commented

2026-08-03 11:09:26 +00:00

Author

Owner

/oc rerun

👀 1

lambadalambda commented

2026-08-03 11:11:57 +00:00

Author

Owner

/oc rerun

lambadalambda commented

2026-08-03 11:15:25 +00:00

Author

Owner

/oc rerun

lambadalambda commented

2026-08-03 11:17:54 +00:00

Author

Owner

/oc rerun

lambadalambda commented

2026-08-03 11:35:09 +00:00

Author

Owner

/oc rerun

lambadalambda added 1 commit

2026-08-03 11:49:45 +00:00

docs: archive OpenCode review pilot

opencode/review OpenCode execution completed

Details

1a04372949

lambadalambda commented

2026-08-03 11:49:53 +00:00

Author

Owner

/oc rerun

opencode-fix-bot commented

2026-08-31 14:31:24 +00:00

First-time contributor

OpenCode Review (rerun)

Rerun for /oc rerun (comment 117412). Reviewed master @ 81c7586 → issue/109-opencode-review-bot-pilot @ 1a04372 (21 commits, 64 files, +5417/−333).

Verification method: static review of the full base-to-head diff. Tests were not executed in this run (sandbox has node 22; the project requires node >=24 and Playwright/docker integration instances), so runtime findings below are code-traced, not reproduced.

Findings

High

Load-more pages appended during catch-up are misclassified as newer posts, corrupting timeline ordering — src/lib/pleroma/timeline-state.ts (reconcileTimelineCatchUp) + src/routes/app/[...path]/+page.svelte

reconcileTimelineCatchUp assumes every item in data that is not in initialDataIds is newer than the catch-up baseline:

const previousData = data.filter((item) => initialDataIds.has(item.id));
const concurrentData = data.filter((item) => !initialDataIds.has(item.id));

But loadMoreHomeTimeline is not gated on the catch-up mutex (it only checks loadMoreStatus === 'loading') and appends older posts to the end of data (data: mergeTimelineItems(homeTimelineState.data, posts)). Those appended posts land in concurrentData, which (a) forces the insert branch — if (insertImmediately || overlapsConcurrentData || concurrentData.length > 0) — silently flushing the "N new posts" pill into the timeline even when insertImmediately is false, and (b) places them inside orderedNew, which is prepended above the baseline: older pages render above the baseline and above genuinely new posts. The same applies to the public-timeline variants. Suggested fix: suppress load-more while a catch-up is in flight, or make concurrentData position-aware (items above the baseline index only).

Medium

Overlap branch flushes queued pill posts even when insertion is declined — src/lib/pleroma/timeline-state.ts (reconcileTimelineCatchUp, overlap branch)

In the overlap branch's non-insert path, queuedBeforeCatchUp (posts sitting unseen in the "N new posts" pill before catch-up started) are merged directly into rebuilt data:

const reconciledData = prependTimelineItems(dataAfterOverlap, mergeTimelineItems(mergeTimelineItems(dataBeforeOverlap, incomingFromOverlap), queuedBeforeCatchUp));

and are excluded from the returned newerPosts. In the symmetric no-overlap branch the same posts correctly stay in newerPosts when not inserting. One of the two behaviors must violate the insertImmediately contract; there is no e2e covering overlap + pending pill + auto-insert off.

Low

Dead predicate — timeline-state.ts: const overlapsConcurrentData = concurrentData.some((item) => incomingIds.has(item.id)) only executes when overlappingDataIndex === -1 (no element of data is in incomingIds), so it can never be true. The guard reduces to insertImmediately || concurrentData.length > 0; the line misleads readers about where concurrent overlap is handled.
Catch-up loop always over-fetches one page — timeline-catch-up.ts: with sinceId set, Mastodon-style APIs return a next link on every non-empty page, so after the last full page the loop always issues one extra (empty) request; termination relies on the server omitting next for empty pages, with the repeated-cursor throw as the only backstop. Breaking on page.items.length === 0 would bound it deterministically.
Watermark advances before dispatch — +page.svelte (checkHomeTimelineForNewPosts, and the public variant): homeTimelineFallbackSinceId = catchUp.newestId ?? ... runs before the status === 'empty' | 'success' dispatch. If a concurrent reload leaves the state 'loading', the catch-up result is discarded while the watermark already advanced past those posts (benign today because a successful reload overwrites both, but the invariant would be airtight if moved inside the branches).
CompactMediaPreview.svelte: (1) handlePhotoKeydown calls event.stopPropagation() on Escape unconditionally, even when no preview is open — swallows the single keyboard path for closing the header notifications popover when focus is on a trigger; early-return when fullPhoto is null (as EmojiPicker.svelte/FocusedPost.svelte do). (2) The video-sampling action guards if (!src || node.src) return; and the {#each} is unkeyed, so a refresh that swaps a video attachment keeps sampling the old source and the sticky ready class hides fallback text prematurely. (3) The floating full-image preview has no touch dismissal (closes only on mouseleave/blur/Escape).
Unbounded session-lifetime growth — +page.svelte: statusEngagementSnapshots, statusEngagementNotificationArrivals, and notificationEngagementSignatures/seenNotificationEngagementSignatures are cleared only on session invalidation, while the persistent app shell keeps adding an entry per loaded status/notification. A slow leak, bounded by distinct ids.
Bookmarks action plumbing is half-migrated — +page.svelte: bookmarks fav/boost errors are recorded under route 'bookmarks' but no error UI exists there (silent rollback); the reaction picker, poll voting, and inline reply on bookmarks still pass 'home' as origin, so a picked reaction toggles scope 'home' and replies set state that never renders. Largely pre-existing (everything was 'home' before), but the new error plumbing makes the gap visible.

Done well

Stream lifecycle hygiene is excellent: the new openTimeoutMs path cleans up its timer and detaches handlers, and the generation counter correctly invalidates superseded streams.
Dedupe discipline in the catch-up pipeline is consistent — no branch drops or duplicates items (case-checked); cursors re-pin sinceId so the multi-page walk cannot cross its window.
No new {@html} anywhere; all user content flows through escaped interpolation or RichText. Sensitive-media previews re-validate against current props before showing originals.
Real accessibility improvements in NotifRow (genuine buttons replacing the role="button" div) and NotificationsPage (tabpanel, labelled controls); loading states gained role="status"/aria-live.

## OpenCode Review (rerun) _Rerun for `/oc rerun` (comment 117412). Reviewed `master` @ `81c7586` → `issue/109-opencode-review-bot-pilot` @ `1a04372` (21 commits, 64 files, +5417/−333)._ **Verification method:** static review of the full base-to-head diff. Tests were not executed in this run (sandbox has node 22; the project requires node >=24 and Playwright/docker integration instances), so runtime findings below are code-traced, not reproduced. ## Findings ### High **Load-more pages appended during catch-up are misclassified as newer posts, corrupting timeline ordering — `src/lib/pleroma/timeline-state.ts` (`reconcileTimelineCatchUp`) + `src/routes/app/[...path]/+page.svelte`** `reconcileTimelineCatchUp` assumes every item in `data` that is not in `initialDataIds` is *newer* than the catch-up baseline: ```ts const previousData = data.filter((item) => initialDataIds.has(item.id)); const concurrentData = data.filter((item) => !initialDataIds.has(item.id)); ``` But `loadMoreHomeTimeline` is not gated on the catch-up mutex (it only checks `loadMoreStatus === 'loading'`) and appends *older* posts to the end of `data` (`data: mergeTimelineItems(homeTimelineState.data, posts)`). Those appended posts land in `concurrentData`, which (a) forces the insert branch — `if (insertImmediately || overlapsConcurrentData || concurrentData.length > 0)` — silently flushing the "N new posts" pill into the timeline even when `insertImmediately` is false, and (b) places them inside `orderedNew`, which is *prepended above the baseline*: older pages render above the baseline and above genuinely new posts. The same applies to the public-timeline variants. Suggested fix: suppress load-more while a catch-up is in flight, or make `concurrentData` position-aware (items above the baseline index only). ### Medium **Overlap branch flushes queued pill posts even when insertion is declined — `src/lib/pleroma/timeline-state.ts` (`reconcileTimelineCatchUp`, overlap branch)** In the overlap branch's non-insert path, `queuedBeforeCatchUp` (posts sitting unseen in the "N new posts" pill *before* catch-up started) are merged directly into rebuilt `data`: ```ts const reconciledData = prependTimelineItems(dataAfterOverlap, mergeTimelineItems(mergeTimelineItems(dataBeforeOverlap, incomingFromOverlap), queuedBeforeCatchUp)); ``` and are excluded from the returned `newerPosts`. In the symmetric no-overlap branch the same posts correctly stay in `newerPosts` when not inserting. One of the two behaviors must violate the `insertImmediately` contract; there is no e2e covering overlap + pending pill + auto-insert off. ### Low - **Dead predicate — `timeline-state.ts`:** `const overlapsConcurrentData = concurrentData.some((item) => incomingIds.has(item.id))` only executes when `overlappingDataIndex === -1` (no element of `data` is in `incomingIds`), so it can never be true. The guard reduces to `insertImmediately || concurrentData.length > 0`; the line misleads readers about where concurrent overlap is handled. - **Catch-up loop always over-fetches one page — `timeline-catch-up.ts`:** with `sinceId` set, Mastodon-style APIs return a `next` link on every non-empty page, so after the last full page the loop always issues one extra (empty) request; termination relies on the server omitting `next` for empty pages, with the repeated-cursor throw as the only backstop. Breaking on `page.items.length === 0` would bound it deterministically. - **Watermark advances before dispatch — `+page.svelte` (`checkHomeTimelineForNewPosts`, and the public variant):** `homeTimelineFallbackSinceId = catchUp.newestId ?? ...` runs before the `status === 'empty' | 'success'` dispatch. If a concurrent reload leaves the state `'loading'`, the catch-up result is discarded while the watermark already advanced past those posts (benign today because a successful reload overwrites both, but the invariant would be airtight if moved inside the branches). - **`CompactMediaPreview.svelte`:** (1) `handlePhotoKeydown` calls `event.stopPropagation()` on Escape unconditionally, even when no preview is open — swallows the single keyboard path for closing the header notifications popover when focus is on a trigger; early-return when `fullPhoto` is null (as `EmojiPicker.svelte`/`FocusedPost.svelte` do). (2) The video-sampling action guards `if (!src || node.src) return;` and the `{#each}` is unkeyed, so a refresh that swaps a video attachment keeps sampling the old source and the sticky `ready` class hides fallback text prematurely. (3) The floating full-image preview has no touch dismissal (closes only on mouseleave/blur/Escape). - **Unbounded session-lifetime growth — `+page.svelte`:** `statusEngagementSnapshots`, `statusEngagementNotificationArrivals`, and `notificationEngagementSignatures`/`seenNotificationEngagementSignatures` are cleared only on session invalidation, while the persistent app shell keeps adding an entry per loaded status/notification. A slow leak, bounded by distinct ids. - **Bookmarks action plumbing is half-migrated — `+page.svelte`:** bookmarks fav/boost errors are recorded under route `'bookmarks'` but no error UI exists there (silent rollback); the reaction picker, poll voting, and inline reply on bookmarks still pass `'home'` as origin, so a picked reaction toggles scope `'home'` and replies set state that never renders. Largely pre-existing (everything was `'home'` before), but the new error plumbing makes the gap visible. ## Done well - Stream lifecycle hygiene is excellent: the new `openTimeoutMs` path cleans up its timer and detaches handlers, and the generation counter correctly invalidates superseded streams. - Dedupe discipline in the catch-up pipeline is consistent — no branch drops or duplicates items (case-checked); cursors re-pin `sinceId` so the multi-page walk cannot cross its window. - No new `{@html}` anywhere; all user content flows through escaped interpolation or `RichText`. Sensitive-media previews re-validate against current props before showing originals. - Real accessibility improvements in `NotifRow` (genuine buttons replacing the `role="button"` div) and `NotificationsPage` (tabpanel, labelled controls); loading states gained `role="status"`/`aria-live`.

opencode/review OpenCode execution completed

Details

This pull request is marked as a work in progress.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin issue/109-opencode-review-bot-pilot:issue/109-opencode-review-bot-pilot

git switch issue/109-opencode-review-bot-pilot

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master

git merge --no-ff issue/109-opencode-review-bot-pilot

git switch issue/109-opencode-review-bot-pilot

git rebase master

git switch master

git merge --ff-only issue/109-opencode-review-bot-pilot

git switch issue/109-opencode-review-bot-pilot

git rebase master

git switch master

git merge --no-ff issue/109-opencode-review-bot-pilot

git switch master

git merge --squash issue/109-opencode-review-bot-pilot

git switch master

git merge --ff-only issue/109-opencode-review-bot-pilot

git switch master

git merge issue/109-opencode-review-bot-pilot

git push origin master

Rows
Columns

WIP: Pilot safe OpenCode pull request reviews #1

Summary

Testing

OpenCode Review

Findings

Medium

Low

Informational

Overall

OpenCode Review (rerun)

Findings

High

Medium

Low

Done well

Checkout

Merge