Conversation
Introduces `silent_streaming` configuration for the Telegram channel (defaulting to `true`). When enabled, the initial "thinking..." placeholder and intermediate streaming updates are sent with `disable_notification: true` to prevent repeated push notifications. Final replies and standalone messages remain noisy. - Added `silent_streaming` to `TelegramConfig` schema. - Updated `TelegramChannel` to respect the setting in `send_draft` and `update_draft`. - Added a `disable_notification` parameter to `send_text_chunks`. - Updated unit tests and added documentation. Closes #931
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Contributor
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server as TelegramChannel
participant TelegramAPI
User->>Server: Starts streaming request
Server->>TelegramAPI: sendMessage (draft, disable_notification: true)
TelegramAPI-->>Server: message_id (silent draft)
Server->>TelegramAPI: editMessage (stream chunk, disable_notification: true)
TelegramAPI-->>Server: edited_message
alt final content ready
Server->>TelegramAPI: sendMessage (final, disable_notification: false)
TelegramAPI-->>User: Notifying final message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/config/schema/channels.rs (1)
105-118:⚠️ Potential issue | 🟡 MinorPlease cover the omitted-field default explicitly.
The backward-compat guarantee here is that older
config.tomlfiles deserialize withsilent_streaming = truewhen the field is absent. I don't see a regression test for that omission case yet, so this can drift silently.As per coding guidelines, "Ship unit tests and coverage for behavior you are adding or changing before building additional features on top".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/config/schema/channels.rs` around lines 105 - 118, Add an explicit unit test that verifies TelegramConfig uses the omitted-field default for silent_streaming (true) when deserializing older configs: serialize or craft a TOML string lacking the silent_streaming key, deserialize it into TelegramConfig (the struct) and assert config.silent_streaming == true; reference the existing default helper default_silent_streaming and the TelegramConfig type in the test name and assertions so the behavior cannot regress silently.
🧹 Nitpick comments (2)
src/openhuman/channels/controllers/ops.rs (1)
237-251: Avoid re-stating Telegram defaults here.This fallback tuple hardcodes
1000andtrue, which is now separate from the schema defaults inTelegramConfig. A sharedDefault/constructor would keep the UI-connect path and the serde-deserialization path from drifting later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/controllers/ops.rs` around lines 237 - 251, The fallback should use the canonical TelegramConfig default instead of re-stating individual defaults; replace the tuple literal with something like let tg_default = crate::openhuman::config::TelegramConfig::default() and then get (tg_default.stream_mode, tg_default.draft_update_interval_ms, tg_default.silent_streaming, tg_default.mention_only) so the UI-path will follow the same defaults as serde deserialization and avoid drifting; update the code around persisted.channels_config.telegram usage to destructure from TelegramConfig::default() when existing is None.src/openhuman/channels/providers/telegram/channel_tests.rs (1)
1386-1395: Consider moving new tests into a smaller dedicated module.This file is already very large, so adding more cases here increases maintenance overhead. A focused split (e.g., streaming-related tests) would improve navigability.
As per coding guidelines,
src/**/*.rs: Source files should be ≤ ~500 lines; split modules when growing to improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/telegram/channel_tests.rs` around lines 1386 - 1395, Move the new streaming-related test out of the very large channel_tests.rs into a small dedicated test module to reduce file size and improve navigability: create a new test module (e.g., streaming_tests) and copy the test function silent_streaming_is_configurable there, keeping the same contents that call TelegramChannel::new and with_streaming(StreamMode::Partial, ...); in the new module add #[cfg(test)] and either use super::* or explicitly import TelegramChannel and StreamMode so the test compiles, and remove the original test from the large file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/channels/providers/telegram/channel_tests.rs`:
- Around line 76-77: Run rustfmt (cargo fmt) and reformat all with_streaming
callsites so they follow rustfmt's chained-method style: move the
.with_streaming(...) onto its own indented line after TelegramChannel::new(...)
and, if the argument list is long, wrap the arguments of with_streaming
(StreamMode::Partial, 750, true) across lines so each argument or logical group
is on its own line; update the callsites that use TelegramChannel::new and
with_streaming (referencing StreamMode::Partial) accordingly and re-run cargo
fmt and cargo check to unblock CI.
In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Around line 1691-1692: The failing CI is due to formatter differences on the
send_text_chunks(...) call sites; run rustfmt (cargo fmt) and reformat the
wrapped calls to the multiline layout expected by the project so the
send_text_chunks invocation and its arguments are each placed/indented per
rustfmt rules (fix both occurrences of send_text_chunks in this module). Ensure
you run cargo fmt at repo root (and app/src-tauri if applicable) and then commit
the reformatted channel.rs so the return self.send_text_chunks(...) lines match
the formatter output.
- Around line 1540-1544: The code creates a silent draft using sendMessage
(building body with "disable_notification": self.silent_streaming) but then
finalizes by calling editMessageText which cannot set notification behavior, so
the final message will always follow Telegram's default; to fix, change the
finalize path in the logic that currently calls editMessageText (referenced by
editMessageText and the draft message_id produced from sendMessage) to instead
delete the silent draft (use deleteMessage on chat_id and message_id) and then
send a new final message with the desired notification setting via sendMessage
(build a fresh body using chat_id, final text, and disable_notification set
appropriately), or if you choose to keep silent through final, skip
editing/sending and leave the draft—update code paths around initial_text, body,
self.silent_streaming, and the finalize routine accordingly.
---
Outside diff comments:
In `@src/openhuman/config/schema/channels.rs`:
- Around line 105-118: Add an explicit unit test that verifies TelegramConfig
uses the omitted-field default for silent_streaming (true) when deserializing
older configs: serialize or craft a TOML string lacking the silent_streaming
key, deserialize it into TelegramConfig (the struct) and assert
config.silent_streaming == true; reference the existing default helper
default_silent_streaming and the TelegramConfig type in the test name and
assertions so the behavior cannot regress silently.
---
Nitpick comments:
In `@src/openhuman/channels/controllers/ops.rs`:
- Around line 237-251: The fallback should use the canonical TelegramConfig
default instead of re-stating individual defaults; replace the tuple literal
with something like let tg_default =
crate::openhuman::config::TelegramConfig::default() and then get
(tg_default.stream_mode, tg_default.draft_update_interval_ms,
tg_default.silent_streaming, tg_default.mention_only) so the UI-path will follow
the same defaults as serde deserialization and avoid drifting; update the code
around persisted.channels_config.telegram usage to destructure from
TelegramConfig::default() when existing is None.
In `@src/openhuman/channels/providers/telegram/channel_tests.rs`:
- Around line 1386-1395: Move the new streaming-related test out of the very
large channel_tests.rs into a small dedicated test module to reduce file size
and improve navigability: create a new test module (e.g., streaming_tests) and
copy the test function silent_streaming_is_configurable there, keeping the same
contents that call TelegramChannel::new and with_streaming(StreamMode::Partial,
...); in the new module add #[cfg(test)] and either use super::* or explicitly
import TelegramChannel and StreamMode so the test compiles, and remove the
original test from the large file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7d479dd-2eaf-4c61-8b90-f420c0c34542
📒 Files selected for processing (9)
docs/channels/telegram.mdsrc/openhuman/channels/commands.rssrc/openhuman/channels/controllers/ops.rssrc/openhuman/channels/providers/telegram/channel.rssrc/openhuman/channels/providers/telegram/channel_tests.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/config/mod.rssrc/openhuman/config/schema/channels.rssrc/openhuman/config/schema/channels_tests.rs
Introduces `silent_streaming` configuration for the Telegram channel (defaulting to `true`). When enabled, the initial "thinking..." placeholder and intermediate streaming updates are sent with `disable_notification: true` to prevent repeated push notifications. Final replies and standalone messages remain noisy. - Added `silent_streaming` to `TelegramConfig` schema. - Updated `TelegramChannel` to respect the setting in `send_draft` and `update_draft`. - Added a `disable_notification` parameter to `send_text_chunks`. - Updated unit tests and added documentation. - Applied formatting fixes to satisfy CI. Closes #931
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/openhuman/channels/providers/telegram/channel.rs (1)
1620-1625:⚠️ Potential issue | 🟡 MinorRemove ineffective
disable_notificationfromeditMessageTextpayload.Telegram's
editMessageTextAPI does not support thedisable_notificationparameter—it's only valid forsendMessageand similar send methods. This parameter will be silently ignored by the API.Note: This doesn't affect functionality since message edits never trigger new push notifications in Telegram regardless. However, including this parameter is misleading and should be removed for clarity.
🧹 Proposed fix
let body = serde_json::json!({ "chat_id": chat_id, "message_id": message_id_parsed, "text": display_text, - "disable_notification": self.silent_streaming, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/channels/providers/telegram/channel.rs` around lines 1620 - 1625, The JSON payload for editing a message includes an unsupported key "disable_notification"; remove that key from the constructed `body` so the object only contains "chat_id", "message_id" (i.e. `message_id_parsed`), and "text" (`display_text`), and stop referencing `self.silent_streaming` in this payload (leave `self.silent_streaming` usage in send methods if needed). Locate the `body` construction where `message_id_parsed`, `chat_id`, `display_text`, and `self.silent_streaming` are used and delete the `"disable_notification": self.silent_streaming` entry so the editMessageText request no longer includes the ineffective parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/openhuman/channels/providers/telegram/channel.rs`:
- Around line 1620-1625: The JSON payload for editing a message includes an
unsupported key "disable_notification"; remove that key from the constructed
`body` so the object only contains "chat_id", "message_id" (i.e.
`message_id_parsed`), and "text" (`display_text`), and stop referencing
`self.silent_streaming` in this payload (leave `self.silent_streaming` usage in
send methods if needed). Locate the `body` construction where
`message_id_parsed`, `chat_id`, `display_text`, and `self.silent_streaming` are
used and delete the `"disable_notification": self.silent_streaming` entry so the
editMessageText request no longer includes the ineffective parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a903c303-b7e7-4c20-8ed6-a23c54edb19a
📒 Files selected for processing (2)
src/openhuman/channels/providers/telegram/channel.rssrc/openhuman/channels/providers/telegram/channel_tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/channels/providers/telegram/channel_tests.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces
silent_streamingconfiguration for the Telegram channel (defaulting totrue). When enabled, the initial "thinking..." placeholder and intermediate streaming updates are sent withdisable_notification: trueto prevent repeated push notifications. Final replies and standalone messages remain noisy.PR created automatically by Jules for task 6151919187494225534 started by @senamakel
Summary by CodeRabbit
New Features
Documentation
Tests