Repository navigation
fix(nicojk): コメント取得が不安定な件の対策 - #35
Merged
Merged
Conversation
レビュアーガイドこの PR では、タイムアウトした NDGR の view/segment ストリームを分離して再試行し、古くなった watch-session URL を更新し、bootstrap WebSocket を独立してクローズすることで、NicoNico コメント取得の堅牢性を高めています。また、コメントのマージとライブレンダラーの再構築を最適化し、nx-jikkyo をデフォルトの取得元に戻し、関連する UI レイアウトを調整しています。 堅牢な NicoNico コメント取得のシーケンス図sequenceDiagram
participant Client as NiconicoCommentClient
participant Watch as WatchPage
participant WS as BootstrapWebSocket
participant View as NDGRViewStream
participant Segment as NDGRSegmentStream
loop reconnect session
Client->>Watch: resolveNiconicoWatchPage()
Client->>WS: openWatchSocket()
WS-->>Client: viewUri
Client->>WS: close()
Client->>View: consumeViewSegment()
View-->>Client: segment URI
Client->>Segment: startSegment()
Segment-->>Client: comment messages
alt timeout or stream failure
Client->>Segment: retry with runNdgrWithRetry()
Client->>View: retry with runNdgrWithRetry()
end
end
ライブコメント取得元選択の状態図stateDiagram-v2
[*] --> NxJikkyoDefault
NxJikkyoDefault --> Niconico: user selects niconico
Niconico --> NxJikkyoDefault: user selects nx-jikkyo
NxJikkyoDefault --> NxJikkyoDefault: invalid or missing setting
効率的なライブコメント更新のフロー図flowchart TD
A[Incoming live comment] --> B{One new comment?}
B -- Yes --> C{Duplicate ID?}
C -- Yes --> D[Keep existing comments]
C -- No --> E[Binary-search sorted position]
E --> F[Insert comment and trim maxCount]
B -- No --> G[Sort and deduplicate merged comments]
F --> H[Schedule target sync]
G --> H
H --> I[Batch player IDs for 50 ms]
I --> J["syncTargetState()"]
ファイル単位の変更
関連する可能性のある Issue
ヒントとコマンドSourcery の操作
エクスペリエンスのカスタマイズダッシュボード にアクセスして、次の操作を行えます。
ヘルプの利用
Original review guide in EnglishReviewer's GuideThe PR hardens NicoNico comment retrieval by separating and retrying timed-out NDGR view/segment streams, refreshing stale watch-session URLs, and closing the bootstrap WebSocket independently; it also optimizes comment merging and live-renderer rebuilds, restores nx-jikkyo as the default source, and adjusts related UI layout. Sequence diagram for resilient NicoNico comment retrievalsequenceDiagram
participant Client as NiconicoCommentClient
participant Watch as WatchPage
participant WS as BootstrapWebSocket
participant View as NDGRViewStream
participant Segment as NDGRSegmentStream
loop reconnect session
Client->>Watch: resolveNiconicoWatchPage()
Client->>WS: openWatchSocket()
WS-->>Client: viewUri
Client->>WS: close()
Client->>View: consumeViewSegment()
View-->>Client: segment URI
Client->>Segment: startSegment()
Segment-->>Client: comment messages
alt timeout or stream failure
Client->>Segment: retry with runNdgrWithRetry()
Client->>View: retry with runNdgrWithRetry()
end
end
State diagram for live comment source selectionstateDiagram-v2
[*] --> NxJikkyoDefault
NxJikkyoDefault --> Niconico: user selects niconico
Niconico --> NxJikkyoDefault: user selects nx-jikkyo
NxJikkyoDefault --> NxJikkyoDefault: invalid or missing setting
Flow diagram for efficient live comment updatesflowchart TD
A[Incoming live comment] --> B{One new comment?}
B -- Yes --> C{Duplicate ID?}
C -- Yes --> D[Keep existing comments]
C -- No --> E[Binary-search sorted position]
E --> F[Insert comment and trim maxCount]
B -- No --> G[Sort and deduplicate merged comments]
F --> H[Schedule target sync]
G --> H
H --> I[Batch player IDs for 50 ms]
I --> J["syncTargetState()"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
こんにちは—1件の問題を見つけました
AIエージェント向けプロンプト
このコードレビューのコメントに対応してください。
## 個別のコメント
### コメント1
<location path="src/plugins/nicojk/niconico-comment-client.ts" line_range="616-625" />
<code_context>
+ const active = this.activeSegmentTasks.get(segmentUri);
+ if (active?.generation === generation) return;
+ if (active) this.activeSegmentTasks.delete(segmentUri);
+ if (this.seenSegmentUris.has(segmentUri)) return;
this.seenSegmentUris.add(segmentUri);
+
+ let task: Promise<void>;
+ task = this.consumeSegment(segmentUri, vposBaseTime, signal, generation)
</code_context>
<issue_to_address>
**issue (bug_risk):** セグメントのURIが、取得に成功する前に `seenSegmentUris` に追加され、すべての再試行が失敗した場合にも削除されません。後でビューストリームが同じURIを再度通知すると、`startSegment` は取得せずに終了するため、そのセグメントのコメントが現在のセッション中ずっと失われます。
**Triggers:** セグメントの取得が `maxNumberOfRetry` 回の再試行を経ても失敗し続け、後のビュー応答で同じセグメントURIが再度通知された場合。
**Suggested fix:** セグメントタスクが再試行を使い果たした時点で、より新しいタスクがすでにそのURIを取得していない場合に限り、`seenSegmentUris` からURIを削除してください。
```suggestion
.catch((error) => {
if (this.activeSegmentTasks.get(segmentUri)?.task === task) {
this.seenSegmentUris.delete(segmentUri);
}
if (!this.isIntentionalFailure(error, signal, generation)) {
console.error("[NicoJK] NicoNico comment segment failed", error);
}
})
.finally(() => {
if (this.activeSegmentTasks.get(segmentUri)?.task === task) {
this.activeSegmentTasks.delete(segmentUri);
}
});
```
</issue_to_address>Sourceryによる評価
人間によるレビューが必要です。 先に対応すべき指摘が1件あり、明示的な設定がないユーザーに対するデフォルトのライブコメントソースを変更するとともに、ストリーミングクライアントの再試行、タイムアウト、再接続の動作を大幅に書き換えています。新しいソースや接続ポリシーに問題がある場合、ユーザーは直ちに欠落した、または誤ったコメントを受け取る可能性があります。また、デフォルトの決定は該当するすべてのユーザーに影響します。元に戻すことで新しい動作は停止できますが、ユーザーが保存した可能性のある設定が自動的に取り消されるわけではありません。
ブロッキングとなる指摘: src/plugins/nicojk/niconico-comment-client.ts:625
もっと役立つレビューにご協力ください！各コメントの 👍 または 👎 をクリックしていただければ、そのフィードバックをレビューの改善に活用します。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugins/nicojk/niconico-comment-client.ts" line_range="616-625" />
<code_context>
+ const active = this.activeSegmentTasks.get(segmentUri);
+ if (active?.generation === generation) return;
+ if (active) this.activeSegmentTasks.delete(segmentUri);
+ if (this.seenSegmentUris.has(segmentUri)) return;
+ this.seenSegmentUris.add(segmentUri);
+
+ let task: Promise<void>;
+ task = this.consumeSegment(segmentUri, vposBaseTime, signal, generation)
</code_context>
<issue_to_address>
**issue (bug_risk):** A segment URI is added to `seenSegmentUris` before its fetch succeeds and is never removed when all retries fail. If the view stream announces that URI again later, `startSegment` returns without fetching it, so comments from that segment are permanently lost for the current session.
**Triggers:** When a segment fetch continues to fail through `maxNumberOfRetry` and the same segment URI is announced again by a later view response.
**Suggested fix:** Remove the URI from `seenSegmentUris` when the segment task exhausts its retries, unless a newer task has already claimed it.
```suggestion
.catch((error) => {
if (this.activeSegmentTasks.get(segmentUri)?.task === task) {
this.seenSegmentUris.delete(segmentUri);
}
if (!this.isIntentionalFailure(error, signal, generation)) {
console.error("[NicoJK] NicoNico comment segment failed", error);
}
})
.finally(() => {
if (this.activeSegmentTasks.get(segmentUri)?.task === task) {
this.activeSegmentTasks.delete(segmentUri);
}
});
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this changes the default live-comment source for users without an explicit setting and substantially rewrites the streaming client’s retry, timeout, and reconnection behavior. If the new source or connection policy is wrong, users can immediately receive missing or incorrect comments and the default decision affects every such user; reverting stops the new behavior but does not automatically undo any persisted settings users may have adopted.
Blocking findings: src/plugins/nicojk/niconico-comment-client.ts:625
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
WKWebViewからのmpnリクエストが時々詰まってしまうようで、原因がよくわかっていない。これの対策のために動作を公式に寄せてみたり、無音のメディアを流してみたり、タイムアウトを短くしたりした。結局あんまりよくならなかったので取得元のデフォルトをnx-jikkyoに戻した。
Sourceryによる概要
NicoNicoのライブコメント取得とレンダリングを安定化し、nx-jikkyoをデフォルトのソースに変更しました。
新機能:
バグ修正:
改善:
テスト:
Original summary in English
Sourceryによる概要
NicoNicoのライブコメントの取得とレンダリングを安定化し、デフォルトのソースをnx-jikkyoに変更しました。
新機能:
バグ修正:
機能強化:
テスト:
Original summary in English
Summary by Sourcery
Stabilize NicoNico live-comment retrieval and rendering while switching the default source to nx-jikkyo.
New Features:
Bug Fixes:
Enhancements:
Tests: