Conversation
同一アカウントでメンション/ダイレクトカラムと通知カラムが併存すると、 両者が別々に main チャンネルを張ろうとし、Misskey の shouldShare 仕様で 2 本目が黙殺されて後から張った側 (通常は通知カラム) にイベントが届かな かった (#984)。 - ingest_stream_event: stream-notification / stream-mention は query_ids_by_subscription の 1:1 マップではなく (account_id, 種別) → QueryKey で対象 query を直接解決する (NoteCaptureUpdated と同じ アカウント単位イベントの型) - query_subscribe_mentions / notifications: attach_shared_stream_subscription で snapshot にだけ購読 ID を載せ、配送マップには登録しない。これにより 片方の close / suspend がもう片方の配送を巻き添えにしない - notecli を fe0ceed に bump (main 購読の per-account dedup + unsubscribe/suspend の no-op 化)。メンションカラムのサスペンドで OS 通知・未読バッジまで止まる問題 (#1002) も同時に解消 - Stream Inspector: 通知/メンションカラムは同じ共有 subscription id を 持つようになる (従来は負けた側の id がどのイベントとも一致せず フィルタが実質壊れていた) refs #984, #1002 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
notecli#58 (main チャンネル dedup) が develop にマージされたため、 feature ブランチ commit への pin を merge commit 2d7305ea に更新。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
長時間起動しても劣化しない、という前提に対して破れていた経路を塞ぐ。 - 画像プレースホルダ (blurhash → data URL): 上限も削除も無く、スクロール した数だけ残っていた。共通基盤に載せる (新ノブ blurhashCacheMax) - 絵文字辞書: 定義・既定値・設定 UI まで揃っていた emojiCachePerHost / emojiPersistPerHost が実装のどこからも参照されておらず、効いているように 見えて何も起きない死にノブだった。実際に配線する - 絵文字辞書の host 数: 連合先の数だけ増え続けていた。新ノブ emojiCacheHosts で頭打ちにし、落とした host の付随状態も一緒に捨てる (辞書だけ刈っても付随状態が残れば同じ速度で増える) - 通知カラムのリアクション URL: キーに通知 ID を含むのに上限が無く、 カラムを閉じるまで消えなかった。通知カラムは開きっぱなしにされる面 Refs #987 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
取得に失敗した URL の記録は、期限判定が読み取り時にしか無く、期限切れの エントリを消す経路が存在しなかった。失敗した URL の数だけ単調増加する。 記録のたびに期限切れを掃き、それでも収まらなければ古い順に落として上限で 頭を打たせる。4xx は 24 時間保持するので、掃除だけでは頭打ちにならない。 Refs #987 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
個別に直して回るのではなく、同じ事故が再発しない形にする (#895 と同じ思想 — 守るかどうかを人間の注意力に委ねない)。 - パフォーマンス設定の全ノブについて、実装のどこかから読まれているかを 検査する。定義・既定値・設定 UI まで揃っているのに参照されていない ノブは、ユーザーから見ると効いているように見えて何も起きない - モジュールスコープの空 Map/Set を全数棚卸しし、なぜ有界なのかを添えた 表と突き合わせる。新しく置かれたらここで落ちて、共通基盤に載せるか 理由を書くかを迫る。表に実在しないエントリが残っていても落とす Refs #987 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
📖Doc
API surface diff外部アプリ向け API 面 ( Full diffdiff --git a/src-tauri/openapi.json b/src-tauri/openapi.json
index e877739..68e0f71 100644
--- a/src-tauri/openapi.json
+++ b/src-tauri/openapi.json
@@ -6,7 +6,7 @@
"license": {
"name": "MIT"
},
- "version": "1.42.6"
+ "version": "1.42.7"
},
"paths": {
"/api": {
|
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change makes Misskey ChangesAccount-scoped main-channel routing
Bounded cache controls
Release metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Misskey
participant QueryRuntime
participant NotificationQuery
participant MentionQuery
Misskey->>QueryRuntime: stream-notification event
QueryRuntime->>NotificationQuery: resolve by account and event type
Misskey->>QueryRuntime: stream-mention event
QueryRuntime->>MentionQuery: resolve by account and event type
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Deploying notedeck with
|
| Latest commit: |
7777056
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d383bcaa.notedeck-d3a.pages.dev |
| Branch Preview URL: | https://develop.notedeck-d3a.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/emojis.ts (1)
83-85: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winApply
emojiCachePerHoston every lookup write path.
set()enforces the per-host limit, but restoration and stream updates bypass it. A large persisted record or manyemojiAddedevents can grow one host lookup without bound.
src/stores/emojis.ts#L83-L85: Trimentry.emojistoemojiCachePerHostbefore inserting it intomap.src/stores/emojis.ts#L300-L316: EnforceemojiCachePerHostafter applying server changes and before assigningnextCache.🤖 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/stores/emojis.ts` around lines 83 - 85, Apply emojiCachePerHost at both lookup write paths in src/stores/emojis.ts: lines 83-85, trim each restored entry.emojis before map.set; and lines 300-316, enforce the same limit after applying server changes and before assigning nextCache. Update the relevant restoration and stream-update logic while preserving existing set() behavior.
🧹 Nitpick comments (1)
tests/lint/moduleCaches.test.ts (1)
115-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude exported module-scope collections in the audit.
export const cache = new Map()does not matchEMPTY_COLLECTION. An exported unbounded cache can bypass the required allowlist check.Proposed fix
- /^(?:const|let) ([A-Za-z_$][\w$]*)(?:\s*:[^=]+?)? = new (?:Map|Set|WeakMap|WeakSet)(?:<[\s\S]*?>)?\(\)/gm + /^(?:export\s+)?(?:const|let) ([A-Za-z_$][\w$]*)(?:\s*:[^=]+?)? = new (?:Map|Set|WeakMap|WeakSet)(?:<[\s\S]*?>)?\(\)/gm🤖 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 `@tests/lint/moduleCaches.test.ts` around lines 115 - 116, Update the EMPTY_COLLECTION pattern to optionally accept the export keyword before const or let, so exported module-scope Map, Set, WeakMap, and WeakSet declarations are included in the audit while preserving existing matches.
🤖 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 `@ARCHITECTURE.md`:
- Around line 209-210: Remove future-stale concrete lifecycle counts from
ARCHITECTURE.md: at lines 209-210, refer to the streaming implementation for
main-channel sharing or document the required lint exception; at line 395,
remove the concrete IPC count and reference the command implementation if
needed; at lines 429-430, remove the mapping cardinality and identify
QueryRuntime as the canonical routing behavior; at line 720, describe the
lifecycle condition without a concrete observer count or add the required
exception.
In `@src-tauri/src/query_runtime.rs`:
- Around line 317-320: Update the shared-attachment logic around
source_subscription_id to remove every query_ids_by_subscription entry
containing query_id before returning, including entries for both the previous
and current subscription IDs. Ensure close() cannot leave stale mappings, and
add regression coverage for changed and identical subscription IDs.
In `@src/services/boundedCache.ts`:
- Around line 33-35: Update the maxOf cache-limit normalization to reject
non-finite values, including NaN and Infinity, before applying the minimum of 1.
Ensure malformed performance overrides resolve to a finite bounded limit so the
eviction comparison remains effective.
In `@src/stores/emojis.ts`:
- Around line 121-135: Update refresh() to verify that the host remains cached
after its in-flight fetch completes and before applying the result via set();
discard stale results when forgetHost() has evicted it. Use the existing host
cache state or add a per-host generation token, ensuring an evicted host cannot
be restored or cause a newer host to be evicted.
---
Outside diff comments:
In `@src/stores/emojis.ts`:
- Around line 83-85: Apply emojiCachePerHost at both lookup write paths in
src/stores/emojis.ts: lines 83-85, trim each restored entry.emojis before
map.set; and lines 300-316, enforce the same limit after applying server changes
and before assigning nextCache. Update the relevant restoration and
stream-update logic while preserving existing set() behavior.
---
Nitpick comments:
In `@tests/lint/moduleCaches.test.ts`:
- Around line 115-116: Update the EMPTY_COLLECTION pattern to optionally accept
the export keyword before const or let, so exported module-scope Map, Set,
WeakMap, and WeakSet declarations are included in the audit while preserving
existing matches.
🪄 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: 9a2523f3-3eef-42df-a5e9-b9f8c4743f23
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
ARCHITECTURE.mdpackage.jsonsrc-tauri/Cargo.tomlsrc-tauri/openapi.jsonsrc-tauri/src/image_cache.rssrc-tauri/src/query_runtime.rssrc-tauri/tauri.conf.jsonsrc/components/deck/DeckNotificationColumn.vuesrc/components/deck/DeckStreamInspectorColumn.vuesrc/defaults/performance.json5src/services/boundedCache.test.tssrc/services/boundedCache.tssrc/stores/emojis.dom.test.tssrc/stores/emojis.tssrc/stores/performance.tssrc/stores/performanceData.tssrc/utils/blurhashDataUrl.tstests/lint/moduleCaches.test.tstests/lint/perfConfigWiring.test.ts
Comment on lines
+209
to
+210
| **main チャンネルは suspend / unsubscribe の対象外** (#984): Misskey の `main` は `shouldShare` チャンネルで **1 WS 接続に 1 本しか張れない**（2 本目の connect はサーバーが黙って無視する）。通知・メンション・OS 通知・未読バッジがすべてこの 1 本にぶら下がるため、main の寿命はカラム（query）ではなく**アカウントセッション**に属する。`StreamingManager` は main をアカウント単位で dedup し、main への `unsubscribe` / `suspend_subscription` を no-op にする。解放経路は `disconnect` のみ。 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace concrete lifecycle counts with canonical references or add documented exceptions.
ARCHITECTURE.md#L209-L210: Refer to the streaming implementation for the main-channel sharing constraint, or add the required preceding lint-disable reason.ARCHITECTURE.md#L395-L395: Remove the concrete IPC count. Refer to the command implementation if the detail is required.ARCHITECTURE.md#L429-L430: Remove the concrete mapping cardinality. Refer toQueryRuntimeas the canonical routing behavior.ARCHITECTURE.md#L720-L720: Describe the lifecycle condition without a concrete observer count, or add the required exception.
As per coding guidelines, root Markdown must not contain future-stale concrete numbers and must refer to the canonical file.
📍 Affects 1 file
ARCHITECTURE.md#L209-L210(this comment)ARCHITECTURE.md#L395-L395ARCHITECTURE.md#L429-L430ARCHITECTURE.md#L720-L720
🤖 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 `@ARCHITECTURE.md` around lines 209 - 210, Remove future-stale concrete
lifecycle counts from ARCHITECTURE.md: at lines 209-210, refer to the streaming
implementation for main-channel sharing or document the required lint exception;
at line 395, remove the concrete IPC count and reference the command
implementation if needed; at lines 429-430, remove the mapping cardinality and
identify QueryRuntime as the canonical routing behavior; at line 720, describe
the lifecycle condition without a concrete observer count or add the required
exception.
Source: Coding guidelines
Comment on lines
+317
to
+320
| if entry.source_subscription_id.as_ref() != Some(&subscription_id) { | ||
| entry.source_subscription_id = Some(subscription_id); | ||
| entry.revision = entry.revision.saturating_add(1); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Remove the previous delivery mapping during shared attachment.
If a query was previously attached with attach_stream_subscription, this method replaces source_subscription_id but leaves its old query_ids_by_subscription entry. Events from the old subscription can still update this query. Later close() removes only the new shared ID, so the old map entry remains stale.
Remove every subscription-map entry that points to query_id before returning. Cover both changed IDs and identical IDs in a regression test.
🤖 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-tauri/src/query_runtime.rs` around lines 317 - 320, Update the
shared-attachment logic around source_subscription_id to remove every
query_ids_by_subscription entry containing query_id before returning, including
entries for both the previous and current subscription IDs. Ensure close()
cannot leave stale mappings, and add regression coverage for changed and
identical subscription IDs.
CodeRabbit レビュー (#1007) の指摘対応: - boundedCache: 非有限の上限 (NaN / Infinity) を 1 に丸める。 performance.json5 は手編集可能で JSON5 は NaN/Infinity リテラルを 許すため、Math.max(1, NaN) 経由で eviction が無効化されていた - emojis: localStorage 復元と emojiAdded push 反映が emojiCachePerHost を 素通りしていた経路に上限を適用 - emojis: refresh() のフェッチ完了が host 追い出しと交差したとき、 追い出し済み host を復活させない Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
CodeRabbit 指摘への対応 (7777056): 対応した (3 件)
見送った (3 件・理由つき)
|
Merged
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.
変更内容
修正
追加
その他
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Release