Skip to content

fix(streaming): polling 稼働中の connect が WebSocket を復活させないようにする - #59

Merged
hitalin merged 4 commits into
developfrom
fix/connect-respects-polling-mode
Aug 7, 2026
Merged

fix(streaming): polling 稼働中の connect が WebSocket を復活させないようにする#59
hitalin merged 4 commits into
developfrom
fix/connect-respects-polling-mode

Conversation

@hitalin

@hitalin hitalin commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

なぜ

notedeck のフロントはカラムのマウントや復帰のたびに stream_connect を無条件に呼ぶ。polling モード中でもこの呼び出しが WS 接続を張り直してしまい、永続化されたモードと実動作が乖離する (notedeck-dev/notedeck#1004)。

何を

StreamingManager::connect の意味を「ストリームを現在のモードで確保する」に揃える。polling タスクがそのアカウントのストリームを供給中なら、Connected を emit して即 return し、WS は張らない。WS へ戻す唯一の経路は set_mode("realtime")

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Prevented unnecessary WebSocket reconnections when polling is already active.
    • Preserved the existing polling connection state during repeated connection attempts.

hitalin and others added 3 commits August 4, 2026 08:50
フロントはカラムのマウントや復帰のたびに stream_connect を無条件に呼ぶため、
polling モード中でも WS 接続が黙って張り直されていた (notedeck#1004)。
connect の意味を「ストリームを現在のモードで確保する」に揃え、polling が
供給中なら Connected を emit して即 return する。WS へ戻す唯一の経路は
set_mode("realtime")。

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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 Plus

Run ID: e0801f5c-e931-4dba-b4cd-eecee2aebcc3

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
📝 Walkthrough

Walkthrough

StreamingManager::connect now detects active polling, reports Connected, and skips WebSocket creation. A regression test verifies that polling remains active and disconnect still works.

Changes

Polling connection guard

Layer / File(s) Summary
Preserve polling during connect
src/streaming.rs
connect returns early when polling is active, emits Connected, and avoids creating a WebSocket. The asynchronous regression test verifies the connection state and clean disconnection.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: preventing connect from reviving WebSocket while polling is active.
✨ 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/connect-respects-polling-mode

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/streaming.rs`:
- Around line 483-487: Update the Connected status path to dispatch the
StreamEvent::Status through EventBus as well as self.emitter by using the
existing emit_both helper with the relevant emitter and event_bus references,
ensuring SSE and event-bus consumers receive the status.
🪄 Autofix

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 Plus

Run ID: 3c29c534-91d4-40bc-98e9-0824f4b7db48

📥 Commits

Reviewing files that changed from the base of the PR and between 272edae and 4d61fb2.

📒 Files selected for processing (1)
  • src/streaming.rs

Comment thread src/streaming.rs
Comment on lines +483 to +487
self.emitter
.emit(StreamEvent::Status(Box::new(StreamStatusEvent {
account_id: account_id.to_string(),
state: StreamConnectionState::Connected,
})));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

ast-grep outline src/streaming.rs --items all --match 'StreamingManager|EventBus'
fd -i '^event_bus\.rs$' . -x ast-grep outline {} --items all
rg -n -C 4 --glob '*.rs' 'StreamEvent::Status|StreamStatusEvent|EventBus|FrontendEmitter' .

Repository: notedeck-dev/notecli

Length of output: 24227


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Read focused streaming/event_bus/server sections and run read-only behavioral checks.
sed -n '1,90p' src/streaming.rs
sed -n '440,700p' src/streaming.rs
sed -n '1720,1940p' src/streaming.rs
sed -n '720,815p' src/http_server.rs
rg -n --glob '*.rs' 'stream-status|s:.*stream-status|sse_event_type|payload_value|event_bus\.send|EventBusEmitter|StreamStatusEvent' .
python3 - <<'PY'
from pathlib import Path
import re
p=Path('src/streaming.rs')
s=p.read_text()
checks={
    'line483_emitter_status': bool(re.search(r'(?ms)$', re.sub(r'//.*','',s)[460:490] )), # placeholder
    'emit_both_contains_polling_capture_comment': 'status / polling 由来 capture は従来どおり emitter のみ' in s,
    'event_bus_send_is_sse_json_payload': 'let data = serde_json::to_string(&event)?' in p.read_text(),
    'status_event_type': 'stream-status' in s and 'Status(Box<StreamStatusEvent>)' in s,
}
for k,v in checks.items():
    print(f'{k}={v}')
PY

PY

Repository: notedeck-dev/notecli

Length of output: 28534


Connected status を EventBus 経由で配信してください。

このコードパスは self.emitter.emit(...) のみを実行し、EventBus へ送信しません。SSE/イベントバス消費者は StreamEvent::Status を受信できません。Statusemit_both(emitter, &event_bus, ...) に移してください。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/streaming.rs` around lines 483 - 487, Update the Connected status path to
dispatch the StreamEvent::Status through EventBus as well as self.emitter by
using the existing emit_both helper with the relevant emitter and event_bus
references, ensuring SSE and event-bus consumers receive the status.

Source: Coding guidelines

@hitalin
hitalin changed the base branch from main to develop August 7, 2026 05:10
@hitalin
hitalin merged commit af25bc0 into develop Aug 7, 2026
5 checks passed
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