Skip to content

fix: repair slide auto-sync in PowerPoint add-in#23

Merged
EinfachMxrc merged 1 commit into
mainfrom
fix/slide-sync
Apr 10, 2026
Merged

fix: repair slide auto-sync in PowerPoint add-in#23
EinfachMxrc merged 1 commit into
mainfrom
fix/slide-sync

Conversation

@EinfachMxrc

@EinfachMxrc EinfachMxrc commented Apr 10, 2026

Copy link
Copy Markdown
Owner

Problem

Die „Aktuelle Folie" im Add-in blieb auf 1 stehen, auch wenn man in PowerPoint auf Folie 2 wechselte.

Ursache

Ein useEffect synchronisierte lastKnownSlide ständig mit sessionData.session.currentSlide (dem DB-Wert). Sobald die Bridge Folie 2 erkannte und die Convex-Mutation startete, konnte ein zwischenzeitliches Convex-Update den UI-Wert auf 1 zurücksetzen – bevor die Mutation abgeschlossen war. Da die Bridge lastReportedSlide = 2 gespeichert hatte, meldete sie Folie 2 nicht nochmal → UI blieb auf 1 hängen.

Fix

  • Neuer State bridgeSlide (nur von Bridge/Manualbuttons beschrieben)
  • lastKnownSlide ist jetzt ein derived value: bridgeSlide ?? DB-Wert ?? 1
  • Der DB-Wert dient nur noch als Fallback (kein aktiver Bridge-Wert vorhanden, z.B. beim ersten Laden oder nach Session-Wechsel)
  • Zusätzlich: try-catch um getSlideCountAsync im officeBridge, um hängende Promises zu verhindern wenn die API nicht verfügbar ist

Test

  1. Add-in in PowerPoint laden, Session auswählen
  2. Auf Folie 2 wechseln → „Aktuelle Folie" springt automatisch auf 2
  3. Session wechseln → zeigt DB-Wert der neuen Session bis Bridge synct

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced PowerPoint slide display consistency when switching between sessions
    • Improved stability of slide information retrieval with better error handling
    • Fixed slide state management to properly prioritize real-time slide detection

The sessionData useEffect was overwriting the bridge-detected slide number
with the stale DB value before the Convex mutation could complete.
Since the bridge guards against re-reporting the same slide, the UI got
stuck at the old value until an unrelated Convex push arrived.

Fix: track bridge slide in separate state (bridgeSlide), derive
lastKnownSlide as bridgeSlide ?? DB value. The DB value is now only
used as fallback when the bridge has not yet reported anything (e.g.
on initial load or after switching sessions).

Also wrap getSlideCountAsync in try-catch to prevent a hanging Promise
if the API is unavailable in certain Office.js environments.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
handout-powerpoint-addin Building Building Preview, Comment Apr 10, 2026 11:22pm

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c423b959-9894-480e-acae-eb39ef6a2fb8

📥 Commits

Reviewing files that changed from the base of the PR and between ea3e5e5 and 56e1b51.

📒 Files selected for processing (2)
  • apps/web/src/components/powerpoint/PowerPointAddinClient.tsx
  • apps/web/src/lib/powerpoint/officeBridge.ts

📝 Walkthrough

Walkthrough

The changes refactor how PowerPoint slide state is managed by introducing a bridge-sourced slide state that takes precedence over database values, with fallback to server state after session switches. Error handling is added to the Office Bridge's slide-count retrieval to gracefully handle synchronous exceptions.

Changes

Cohort / File(s) Summary
Slide State Prioritization
apps/web/src/components/powerpoint/PowerPointAddinClient.tsx
Replaced lastKnownSlide state with nullable bridgeSlide. Added useEffect to clear bridgeSlide on session ID changes. Introduced derived lastKnownSlide value that prioritizes bridgeSlide over DB-backed sessionData.session.currentSlide. Both Office Bridge and manual slide handlers now write to bridgeSlide.
Error Handling for Async Operations
apps/web/src/lib/powerpoint/officeBridge.ts
Wrapped getSlideCountAsync() call in try/catch within getCurrentSlideInfo() to handle synchronous throws. On exception, resolves with totalSlides: slideNumber fallback; normal async path still computes from countResult.value or defaults to slideNumber.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 A bridge now whispers slide changes first,
No longer thirsty for the database's thirst,
When errors arise, we catch and forgive,
With fallback values, the slides still live. ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/slide-sync

Comment @coderabbitai help to get the list of available commands and usage tips.

@EinfachMxrc EinfachMxrc merged commit fb45cdf into main Apr 10, 2026
1 of 3 checks passed
@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a race condition in the PowerPoint add-in where the displayed "Current Slide" would revert to slide 1 after switching slides in PowerPoint. The root cause was a useEffect that continuously synced lastKnownSlide from the Convex DB value, which could overwrite a bridge-detected slide change before the mutation completed.

Key changes:

  • Replaces the lastKnownSlide stateful variable with bridgeSlide (nullable), making lastKnownSlide a derived value: bridgeSlide ?? sessionData?.session.currentSlide ?? 1. The bridge value now takes permanent priority over the DB value, eliminating the race.
  • Adds a dedicated useEffect that resets bridgeSlide to null on session switch, allowing the DB value to serve as the fallback until the bridge re-detects the current PowerPoint slide.
  • Wraps getSlideCountAsync in a try-catch inside the getSelectedDataAsync callback, preventing a hanging Promise when that API is unavailable (synchronous throw inside the async callback previously had no catch handler, causing the outer Promise to never resolve).

Confidence Score: 5/5

Safe to merge — the fix is correct, targeted, and introduces no new regressions.

The race condition is accurately diagnosed and the fix is sound: making lastKnownSlide a derived value (bridgeSlide ?? DB ?? 1) ensures the bridge value permanently wins over intermediate DB updates, and the dedicated useEffect on selectedSessionId correctly clears bridgeSlide on session switch so the DB value resumes as the fallback. The try-catch addition in officeBridge.ts prevents hanging Promises when getSlideCountAsync is absent and throws synchronously — a real gap in the previous code. No new logic bugs were found and no pre-existing issues were worsened.

No files require special attention.

Important Files Changed

Filename Overview
apps/web/src/components/powerpoint/PowerPointAddinClient.tsx Core fix: replaces reactive lastKnownSlide state with bridgeSlide (nullable) plus a derived value, preventing the DB value from overwriting a bridge-detected slide before the mutation completes. Session-switch reset is correctly handled via a dedicated effect.
apps/web/src/lib/powerpoint/officeBridge.ts Defensive fix: wraps getSlideCountAsync in a try-catch inside the async callback so synchronous throws (method unavailable) no longer cause a permanently-pending Promise. Fallback correctly uses slideNumber as totalSlides.

Reviews (1): Last reviewed commit: "fix: repair slide auto-sync in PowerPoin..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot mentioned this pull request Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant