Skip to content

feat(discord): desktop Discord Rich Presence (reading/TTS activity)#233

Draft
anishgoyal1108 wants to merge 1 commit into
IReaderorg:masterfrom
anishgoyal1108:feat/discord-rich-presence
Draft

feat(discord): desktop Discord Rich Presence (reading/TTS activity)#233
anishgoyal1108 wants to merge 1 commit into
IReaderorg:masterfrom
anishgoyal1108:feat/discord-rich-presence

Conversation

@anishgoyal1108

Copy link
Copy Markdown

Description

Adds desktop Discord Rich Presence for reading / TTS activity.

IReader takes on no Discord SDK dependency: it publishes the current activity to
~/.cache/IReader/discord_state.json via a small debounced, atomic-rename file
writer, and a companion standalone bridge (ireader-discord) reads the file and
pushes the actual RPC. Deleting the file on Idle clears presence.

  • ActivityStateHolder (common) + nowMillis() expect/actual (android, desktop).
  • DiscordStatePublisher (desktop): writer with a 20s heartbeat so a motionless
    reading session doesn't go stale.
  • TTSActivityBridge (desktop): mirrors v2 TTSController playback into the holder
    so presence shows "Listening" during TTS.
  • AppTab.displayName: stable, locale-independent tab name for the payload.
  • Wiring: Koin singles, eager startup init, browsing-tab hook in MainStarterScreen.
  • New AppPreferences.discordRichPresenceEnabled() gates startup (default on).

Type of Change

  • New feature

Testing

  • Compiled successfully (desktop) ← reviewer to confirm
  • With the companion bridge running, presence updates on tab change and shows
    "Listening" during TTS; closing IReader clears presence

Additional Notes / open questions

  • External bridge: the actual RPC (and the Discord application/client ID) lives
    in the separate ireader-discord Python service, not this repo. Happy to either
    document it as a companion install or reimplement the RPC in-app if preferred.
  • Scoped: browsing + TTS presence are wired here; the reading/perusing presence
    hooks (reader + book-detail VMs) and a Settings UI toggle are intended follow-ons.
  • iOS actual omitted because upstream removed the iOS target.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d8265d4-3946-4400-b223-fe04de60b9a3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Publishes the user's current activity to ~/.cache/IReader/discord_state.json
via a small file-IPC publisher. A companion standalone bridge (ireader-discord)
reads the file and pushes the actual RPC, so IReader takes on no Discord SDK
dependency.

- ActivityStateHolder (common) + nowMillis() expect/actual (android, desktop).
- DiscordStatePublisher (desktop): debounced atomic-rename JSON writer with a
  20s heartbeat; deletes the file on Idle so the bridge clears presence.
- TTSActivityBridge (desktop): mirrors v2 TTSController playback into the holder
  so presence shows "Listening" during TTS.
- AppTab.displayName: stable, locale-independent tab name for the payload.
- Wiring: Koin singles (common holder; desktop publisher+bridge), eager start
  in Main, browsing-tab hook in MainStarterScreen.
- New AppPreferences.discordRichPresenceEnabled() gates startup (default on).

iOS actual dropped (upstream removed the iOS target). Reading/perusing presence
hooks (reader + book-detail VMs) and a Settings UI toggle are follow-ons.
@kazemcodes

kazemcodes commented Jun 6, 2026

Copy link
Copy Markdown
Member

What does this actually do? What's the Idea and need behind this code?

@anishgoyal1108

anishgoyal1108 commented Jun 11, 2026

Copy link
Copy Markdown
Author

What does this actually do? What's the Idea and need behind this code?

It's a toggle that lets users display what they're reading in IReader on Discord via the RPC protocol. I'm not at my computer right now to show what the system service I built in Python looks like, but this is the backend to support anyone who wants to build their own service anyway. In my next reply (likely in a couple of weeks), I will showcase the sample Python service and what the result looks like on Discord.

@kazemcodes

Copy link
Copy Markdown
Member

PR #233 Review: Discord Rich Presence
Overall Verdict: Not ready to merge — several issues need addressing.

What's Good

  • Clean architecture: File-based IPC keeps IReader SDK-free. Well-thought-out separation.
  • Atomic file writes: *.tmp + rename pattern prevents partial reads.
  • 300ms debounce + 20s heartbeat: Sensible for presence updates.
  • TTS priority lock: Correctly prevents reader from clobbering TTS presence.
  • Documentation: DISCORD_RICH_PRESENCE.md is thorough and well-structured.
  • Koin registration pattern: Follows existing codebase conventions.

Critical Issues

  1. CoroutineScope leak in DiscordStatePublisher
    domain/src/desktopMain/kotlin/ireader/domain/services/discord/DiscordStatePublisher.kt creates CoroutineScope(SupervisorJob() + Dispatchers.IO) but:
  • stop() cancels jobs but doesn't cancel the scope
  • No shutdown hook in Main.kt — exitProcess(0) won't call stop()
  • The scope is never cleaned up, leaking coroutines
  1. Missing shutdown cleanup in Main.kt
    desktop/src/main/kotlin/ireader/desktop/Main.kt:143 — onCloseRequest just calls exitProcess(0). The publisher never writes the Idle state / cleans up on app exit. The heartbeat will keep running until process death.
  2. Hardcoded chapterIndex = 1, chapterCount = 1 in TTSActivityBridge
    domain/src/desktopMain/kotlin/ireader/domain/services/discord/TTSActivityBridge.kt:55-56 — This is a known limitation but produces incorrect presence. If chapter info isn't available, it should probably not publish chapter details at all, or the bridge should observe the reader VM's state instead.

Medium Issues
4. Cache directory inconsistency
DiscordStatePublisher uses ~/.cache/IReader but Main.kt:67 uses AppData\Local\IReader (Windows). The existing getCacheDir() at line 351 already handles OS-specific paths. Use it instead of hardcoding.
5. Manual JSON serialization
DiscordStatePublisher.toJson() builds JSON with StringBuilder. This is fragile — a missing escape or field will break the bridge silently. Consider using kotlinx.serialization which the project already has.
6. Default preference value
AppPreferences.discordRichPresenceEnabled() defaults to true, but the feature requires a separate Python bridge most users won't have. Should default to false.
7. ActivityStateHolder in commonMain is wasteful
The holder is registered as a singleton in DomainServices (common), but only desktop has a publisher. Every platform pays the cost of the StateFlow + ttsLocked field. Consider making it desktop-only or using a no-op on other platforms.

Minor Issues
8. AppTab.displayName API change
Adding a constructor parameter to the sealed class changes the API. All subclasses must now pass it. This is fine but should be noted — any external code using AppTab subclasses will break.
9. Swallowed exceptions
Main.kt wraps the eager init in catch (_: Exception) which silently hides Koin resolution failures. At minimum, log a warning.
10. tmpFile.renameTo() fallback isn't atomic
The fallback stateFile.writeText(json) leaves a window where the poller could read a partial file. Minor since the heartbeat rewrites quickly.

Summary
The feature concept is solid but the implementation has lifecycle management issues (scope leak, no shutdown hook), a correctness problem (TTS chapter data), and some design inconsistencies (cache path, default pref). I'd recommend addressing the critical and medium issues before merging.

@kazemcodes

Copy link
Copy Markdown
Member

I research a little about the discord rich peresence, but this feature needs more thinking

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.

2 participants