-
Notifications
You must be signed in to change notification settings - Fork 0
feat: backfill all own_ fields for current_feed received from WS event #207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@szuperaz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors own_fields hydration: removes centralized capabilities cache, renames "capabilities" → "fields", introduces batched own-fields queuing for unseen WS feeds, applies per-feed field updates from ownBatch responses, and updates tests, hooks, and throttling utilities to the new per-field flow. (33 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WS as WebSocket
participant Client as FeedsClient
participant Queue as queueBatchedOwnFields
participant Throttler as ThrottledOwnFields
participant API as ownBatch API
participant Feed as Feed State
Note over WS,Client: Activity event arrives via WebSocket
WS->>Client: activityAdded(activity)
Client->>Client: getOrCreateFeed / shouldAddToActiveFeeds(enrichmentOptions)
alt feed unseen & fromWebSocket
Client->>Queue: queueBatchedOwnFields(feedId)
Queue->>Throttler: schedule batched call (throttled)
end
Throttler->>API: ownBatch([feedIds])
API-->>Throttler: ownBatch(response per feed)
Throttler->>Client: deliver batched own_fields response
Client->>Feed: apply per-feed own_fields to feed.state (fieldsToUpdate-aware)
Note over Feed: UI hooks/selectors pick up updated feed.state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
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. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts:
- Around line 19-23: The code in useOwnCapabilities.ts assumes a string feed is
"groupId:id" without validation; update the branch that handles typeof feed ===
'string' to validate the split result (ensure exactly one colon, both parts
non-empty after trimming, and no extra segments) before calling
client?.feed(groupId, id); if validation fails, avoid calling client.feed and
instead set feed to undefined (or handle via existing error path/log) so invalid
identifiers don't produce incorrect feed objects; reference the variables feed,
feedFromProps, feedFromContext and the call to client?.feed(groupId, id) when
making the change.
🧹 Nitpick comments (3)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
228-249: Good test coverage for throttling behavior.The test correctly validates the throttling mechanism. However, I notice an inconsistency:
The method
setGetBatchOwnCapabilitiesThrottlingInterval(Line 231) still uses "Capabilities" naming while the throttled function uses "Fields" (throttledGetBatchOwnFields). Consider renaming for consistency.Minor: The callbacks are inconsistent - the first returns
undefined(Line 235-236) while the second returnsPromise.resolve()(Line 240). This works but could be cleaner.♻️ Optional: Consider consistent callback style
- client['throttledGetBatchOwnFields']( - [`feed:1`, `feed:2`, `feed:3`], - () => {}, - ); + client['throttledGetBatchOwnFields']( + [`feed:1`, `feed:2`, `feed:3`], + () => Promise.resolve(), + );packages/feeds-client/src/feed/feed.ts (1)
999-1011: Consider potential for duplicate batch requests across feed instances.The
seenCurrentFeedsis maintained per-feed instance. If the same activity (with the samecurrent_feed) arrives via WebSocket to multiple active feed instances, each will queue a batch request for the same feed since they have independentseenCurrentFeedssets.This may be intentional (each feed manages its own hydration), but if deduplication across feeds is desired, consider tracking seen feeds at the client level instead.
packages/feeds-client/src/feeds-client/feeds-client.ts (1)
284-307: Naming inconsistency: method still uses "Capabilities" while internals use "Fields".The method
setGetBatchOwnCapabilitiesThrottlingIntervalretains the "Capabilities" naming while internal variables and the throttled function use "Fields" naming (e.g.,throttledGetBatchOwnFields). Additionally, the error typeUnhandledErrorType.FetchingOwnCapabilitiesOnNewActivityat Line 296 also uses "Capabilities".Consider renaming for consistency, though if this is a public/protected method, it may require a deprecation path for backwards compatibility.
♻️ Proposed rename for consistency
- private setGetBatchOwnCapabilitiesThrottlingInterval = ( + private setGetBatchOwnFieldsThrottlingInterval = ( throttlingMs: number, ) => {Note: The error type
FetchingOwnCapabilitiesOnNewActivitymay need to remain unchanged if it's part of the public API to maintain backwards compatibility.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
packages/feeds-client/__integration-tests__/deferred-own-capabilities-hydration.test.tspackages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.tspackages/feeds-client/__integration-tests__/websocket-connection.test.tspackages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/common/types.tspackages/feeds-client/src/feed/event-handlers/activity/handle-activity-added.tspackages/feeds-client/src/feed/event-handlers/activity/handle-activity-updated.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/utils/throttling/index.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-capabilities.test.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts
💤 Files with no reviewable changes (5)
- packages/feeds-client/integration-tests/websocket-connection.test.ts
- packages/feeds-client/src/feed/event-handlers/activity/handle-activity-updated.ts
- packages/feeds-client/src/utils/throttling/throttled-get-batched-own-capabilities.test.ts
- packages/feeds-client/integration-tests/deferred-own-capabilities-hydration.test.ts
- packages/feeds-client/src/feed/event-handlers/activity/handle-activity-added.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for all code in this repository
Files:
packages/feeds-client/src/common/types.tspackages/feeds-client/src/utils/throttling/index.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories
Files:
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository
Applied to files:
packages/feeds-client/src/common/types.tspackages/feeds-client/src/utils/throttling/index.tspackages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
Applied to files:
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to packages/react-bindings/**/*.{ts,tsx} : Use React hooks and contexts from react-bindings for React SDK integrations
Applied to files:
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to packages/react-bindings/**/*.{ts,tsx} : Support React 18+ and React Native 0.73+ in respective SDKs (react-sdk/ and react-native-sdk/)
Applied to files:
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts
🧬 Code graph analysis (5)
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.ts (1)
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (2)
clearQueuedFeeds(42-44)queueBatchedOwnFields(22-40)
packages/feeds-client/src/feed/feed.test.ts (3)
packages/feeds-client/src/test-utils/response-generators.ts (2)
generateFeedResponse(84-115)generateActivityResponse(135-167)packages/feeds-client/src/feed/feed.ts (1)
feed(271-273)packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (1)
clearQueuedFeeds(42-44)
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts (4)
packages/feeds-client/src/gen/models/index.ts (2)
FeedOwnCapability(2709-2739)FeedOwnCapability(2741-2742)packages/feeds-client/src/feed/feed.ts (3)
currentState(275-277)Feed(162-1024)feed(271-273)packages/feeds-client/src/bindings/react/contexts/StreamFeedsContext.tsx (1)
useFeedsClient(20-22)packages/feeds-client/src/bindings/react/contexts/StreamFeedContext.tsx (1)
useFeedContext(17-19)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
packages/feeds-client/src/common/utils.ts (1)
sleep(1-2)
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (1)
packages/feeds-client/src/utils/throttling/throttle.ts (1)
ThrottledFunction(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-and-test (22.x)
🔇 Additional comments (16)
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts (2)
10-14: LGTM! Selector follows React best practices.The top-level selector definition prevents recreation on every render, and the fallback to
stableEmptyArrayensures referential stability whenown_capabilitiesis undefined.
25-26: LGTM! Clean state access pattern.The
useStateStorecall with optional chaining properly handles the undefined feed case, and the destructuring with fallback provides additional safety.packages/feeds-client/__integration-tests__/docs-snippets/feed-capabilities.test.ts (1)
35-49: LGTM! Per-feed state subscription correctly implemented.The refactor from centralized client state to per-feed state tracking is correctly implemented. The test now derives the feed from
activity.current_feedand subscribes directly to that feed's state, which aligns with the PR's objective to backfill own_ fields at the feed level.packages/feeds-client/src/feed/feed.test.ts (4)
1-7: LGTM! Proper test setup with cleanup utilities.The addition of
afterEachandclearQueuedFeedsensures proper test isolation by resetting the throttling queue between tests.
217-232: LGTM! Mock type correctly extended.The mock type properly includes
throttledGetBatchOwnFieldsto support testing the new WebSocket own fields fetching behavior.
324-361: Excellent test coverage for WebSocket own fields fetching.This test comprehensively verifies the deduplication logic:
- No fetch when activities are not from WebSocket
- No fetch when feed has already been seen
- Fetch triggered only for new feeds from WebSocket events
The test correctly validates the feed argument passed to
throttledGetBatchOwnFields.
363-365: LGTM! Proper test cleanup.The
afterEachcleanup prevents queue state from leaking between tests.packages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.ts (1)
1-141: Comprehensive integration test for deferred own_ fields hydration.This test thoroughly validates the deferred hydration behavior:
- Sets up realistic relationships (follows, followings, memberships)
- Verifies all four own_ field types are hydrated (capabilities, follows, followings, membership)
- Confirms
throttledGetBatchOwnFieldsis called exactly once with correct feed IDs- Includes proper resource cleanup
The test effectively validates the PR's core objective of backfilling own_ fields for feeds received via WebSocket.
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (1)
4-44: LGTM! Consistent terminology update from Capabilities to Fields.The rename from "Capabilities" to "Fields" is consistently applied across all types, constants, and function names. The logic remains unchanged and correct.
packages/feeds-client/src/common/types.ts (1)
9-9: Remove this comment — the field rename concern is based on an incorrect premise.The old field name
query_batch_own_capabilties_throttling_intervaldoes not exist anywhere in the codebase. Onlyquery_batch_own_fields_throttling_intervalis present in types.ts and is consistently used throughout the codebase. This is not a field rename with backwards compatibility implications; the old field name is entirely absent.Likely an incorrect or invalid review comment.
packages/feeds-client/src/utils/throttling/index.ts (1)
1-2: LGTM!The re-export update correctly reflects the rename from "capabilities" to "fields" terminology, maintaining consistency with the broader refactoring in this PR.
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.test.ts (1)
1-76: LGTM! Comprehensive test coverage for the batching logic.The tests thoroughly cover:
- Queuing and clearing feeds via callback
- API limit enforcement (100 feeds max)
- Edge case handling for empty feeds array
The test structure is clean with proper cleanup in
beforeEach.packages/feeds-client/src/feed/feed.ts (2)
1015-1023: LGTM!The
shouldAddToActiveFeedshelper correctly implements the logic from the linked issue (REACT-711): skip fetching own fields whenskip_activity_current_feedorskip_allflags are set.
932-945: LGTM!The backfill logic correctly preserves
current_feedcontext for activities posted to multiple feeds. When a WebSocket event lackscurrent_feedbut the local state has it, the value is propagated to the event for downstream handlers.packages/feeds-client/src/feeds-client/feeds-client.ts (2)
693-702: LGTM!The
ownBatchmethod now directly updates per-feed state from the response, replacing the previous cache-based approach. This is a cleaner pattern that:
- Iterates over response entries safely
- Only updates feeds that exist in
activeFeeds- Applies
ownFieldsdirectly to feed state viapartialNext
152-154: LGTM!The initialization correctly uses the renamed option
query_batch_own_fields_throttling_intervalwith proper fallback to the default constant.
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/feeds-client/src/feed/feed.ts:
- Around line 1000-1013: Remove the debug console.log calls that leak to CI:
delete the two console.log statements referencing 'itt' and 'queueing' near the
logic that computes uninitializedFeeds (the lines that log
feed.currentState.own_capabilities and the array from
uninitializedFeeds.map((feed) => feed.feed)). Keep the surrounding logic intact
(the filter that checks feed.currentState.own_capabilities === undefined and the
call to queueBatchedOwnFields.bind(this.client)({ feeds: ... })); do not alter
variable names like uninitializedFeeds, queueBatchedOwnFields, or this.client.
In @packages/feeds-client/src/utils/check-own-fields-equality.ts:
- Around line 42-50: isOwnCapabilitiesEqual mutates the input arrays by calling
sort() directly on currentState.own_capabilities and newState.own_capabilities;
avoid in-place mutation by creating sorted copies (e.g., use toSorted() if
available or [...array].sort()) and handle undefined values safely before
joining, so update the comparisons in isOwnCapabilitiesEqual to sort copies of
currentState.own_capabilities and newState.own_capabilities (or fallback to
empty arrays) rather than calling sort() on the originals.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/feeds-client/__integration-tests__/deferred-own-fields-hydration.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/utils/check-own-fields-equality.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/feeds-client/integration-tests/deferred-own-fields-hydration.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for all code in this repository
Files:
packages/feeds-client/src/utils/check-own-fields-equality.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/feed/feed.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories
Files:
packages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feed/feed.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
Applied to files:
packages/feeds-client/src/utils/check-own-fields-equality.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/feed/feed.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository
Applied to files:
packages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/feed/feed.ts
🧬 Code graph analysis (4)
packages/feeds-client/src/utils/check-own-fields-equality.ts (1)
packages/feeds-client/src/gen/models/index.ts (1)
FeedResponse(2774-2816)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
packages/feeds-client/src/common/utils.ts (1)
sleep(1-2)
packages/feeds-client/src/feed/feed.test.ts (3)
packages/feeds-client/src/test-utils/response-generators.ts (2)
generateFeedResponse(84-115)generateActivityResponse(135-167)packages/feeds-client/src/feed/feed.ts (1)
feed(270-272)packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (1)
clearQueuedFeeds(42-44)
packages/feeds-client/src/feeds-client/feeds-client.ts (6)
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (3)
ThrottledGetBatchedOwnFields(15-16)DEFAULT_BATCH_OWN_FIELDS_THROTTLING_INTERVAL(18-18)GetBatchedOwnFieldsThrottledCallback(10-13)packages/feeds-client/src/utils/throttling/throttle.ts (1)
throttle(29-123)packages/feeds-client/src/activity-with-state-updates/activity-with-state-updates.ts (1)
feeds(57-59)packages/feeds-client/src/feed/feed.ts (1)
feed(270-272)packages/feeds-client/src/utils/check-own-fields-equality.ts (1)
isOwnCapabilitiesEqual(42-50)packages/feeds-client/src/common/Poll.ts (1)
data(106-108)
🪛 GitHub Actions: Lint and test
packages/feeds-client/src/feed/feed.ts
[error] 1000-1000: yarn lint:all failed. ESLint: Unexpected console statement. (no-console) at line 1000:11.
🪛 GitHub Check: lint-and-test (22.x)
packages/feeds-client/src/feed/feed.ts
[failure] 1005-1005:
Unexpected console statement
[failure] 1000-1000:
Unexpected console statement
🔇 Additional comments (7)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
228-250: LGTM!The throttling test properly verifies that batched own-fields requests are throttled correctly, with appropriate timing checks and mock validation.
packages/feeds-client/src/feed/feed.test.ts (1)
329-358: LGTM!The test properly validates that own fields are fetched for uninitialized feeds when activities arrive via WebSocket, with appropriate cleanup to prevent test pollution.
packages/feeds-client/src/feed/feed.ts (2)
1017-1025: LGTM!The
shouldAddToActiveFeedshelper centralizes the enrichment options checking logic, improving code clarity and maintainability.
975-1015: The uninitialized feed detection logic is already validated by existing tests. The comment at line 1001 explicitly documents the assumption that "own_capabilities can only be undefined if we haven't fetched it yet," and the integration testdeferred-own-fields-hydration.test.tsconfirms thatown_capabilitiesis defined after ownBatch completes. The unit test "should fetch own_ fields if own_capabilities is undefined" validates the detection behavior directly. No changes needed.packages/feeds-client/src/feeds-client/feeds-client.ts (3)
90-92: LGTM!The renaming from "capabilities" to "fields" in the throttling surface is consistent and improves clarity. The throttling setup properly uses trailing-edge throttling to batch requests efficiently.
Also applies to: 130-132, 153-155, 285-306, 377-379, 621-621
694-699: LGTM!The new approach of directly updating per-feed state from
ownBatchresponses eliminates the need for a centralized capabilities cache, simplifying state management.
884-886: State comparison logic looks good.The addition of
isOwnCapabilitiesEqualcheck prevents unnecessary state updates when own_capabilities haven't changed. However, note the array mutation issue flagged incheck-own-fields-equality.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/feeds-client/src/utils/check-own-fields-equality.ts:
- Around line 49-57: The comparison in isOwnCapabilitiesEqual uses
Array.prototype.toSorted() (currentState.own_capabilities?.toSorted() and
newState.own_capabilities?.toSorted()), which is an ES2023 API and may break
runtimes targeting ES2020; replace toSorted() with a compatible approach such as
copying and sorting via [...array].sort() (or add a polyfill/document minimum
Node/browser versions), so update isOwnCapabilitiesEqual to defensively handle
undefined and use [...currentState.own_capabilities].sort().join(',') and
[...newState.own_capabilities].sort().join(',') (or equivalent) to ensure
compatibility.
🧹 Nitpick comments (2)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
384-406: Consider adding timer cleanup to prevent test flakiness.The throttle test relies on timing with
sleep. While the logic is sound, consider usingvi.useFakeTimers()for deterministic control over timing, which would make the test more reliable and faster.♻️ Optional: Use fake timers for deterministic testing
it(`should throttle calls to ownBatch endpoint`, async () => { + vi.useFakeTimers(); vi.spyOn(client, 'ownBatch').mockResolvedValue({ data: {} } as any); const throttleTime = 100; client['setGetBatchOwnFieldsThrottlingInterval'](throttleTime); client['throttledGetBatchOwnFields']( [`feed:1`, `feed:2`, `feed:3`], () => {}, ); expect(client['ownBatch']).toHaveBeenCalledTimes(1); client['throttledGetBatchOwnFields']( [`feed:4`, `feed:5`, `feed:6`], () => {}, ); expect(client['ownBatch']).toHaveBeenCalledTimes(1); - await sleep(throttleTime / 2); + await vi.advanceTimersByTimeAsync(throttleTime / 2); expect(client['ownBatch']).toHaveBeenCalledTimes(1); - await sleep(throttleTime / 2 + 50); // +50 to ensure the trailing call is scheduled + await vi.advanceTimersByTimeAsync(throttleTime / 2 + 50); expect(client['ownBatch']).toHaveBeenCalledTimes(2); + vi.useRealTimers(); });packages/feeds-client/src/feeds-client/feeds-client.ts (1)
288-309: Error type still references "OwnCapabilities" naming.The error type
UnhandledErrorType.FetchingOwnCapabilitiesOnNewActivitystill uses the old "Capabilities" naming while the rest of the code has been renamed to "OwnFields". Consider updating the error type for consistency.♻️ Consider renaming error type for consistency
If the
UnhandledErrorTypeenum is defined in this codebase, consider renaming:- UnhandledErrorType.FetchingOwnCapabilitiesOnNewActivity, + UnhandledErrorType.FetchingOwnFieldsOnNewActivity,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/feeds-client/feeds-client.tspackages/feeds-client/src/utils/check-own-fields-equality.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for all code in this repository
Files:
packages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/utils/check-own-fields-equality.tspackages/feeds-client/src/feeds-client/feeds-client.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories
Files:
packages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feeds-client/feeds-client.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
Applied to files:
packages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/utils/check-own-fields-equality.tspackages/feeds-client/src/feeds-client/feeds-client.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository
Applied to files:
packages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feed/feed.tspackages/feeds-client/src/feeds-client/feeds-client.test.tspackages/feeds-client/src/utils/check-own-fields-equality.tspackages/feeds-client/src/feeds-client/feeds-client.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to packages/react-bindings/**/*.{ts,tsx} : Use React hooks and contexts from react-bindings for React SDK integrations
Applied to files:
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to packages/react-bindings/**/*.{ts,tsx} : Support React 18+ and React Native 0.73+ in respective SDKs (react-sdk/ and react-native-sdk/)
Applied to files:
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts
🧬 Code graph analysis (4)
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts (4)
packages/feeds-client/src/gen/models/index.ts (2)
FeedOwnCapability(2709-2739)FeedOwnCapability(2741-2742)packages/feeds-client/src/feed/feed.ts (3)
currentState(274-276)Feed(162-1035)feed(270-272)packages/feeds-client/src/bindings/react/contexts/StreamFeedsContext.tsx (1)
useFeedsClient(20-22)packages/feeds-client/src/bindings/react/contexts/StreamFeedContext.tsx (1)
useFeedContext(17-19)
packages/feeds-client/src/feed/feed.ts (2)
packages/feeds-client/src/gen/models/index.ts (3)
GetOrCreateFeedResponse(3316-3346)FeedResponse(2774-2816)EnrichmentOptions(2423-2459)packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (1)
queueBatchedOwnFields(22-40)
packages/feeds-client/src/utils/check-own-fields-equality.ts (2)
packages/feeds-client/src/gen/models/index.ts (2)
FollowResponse(3064-3084)FeedResponse(2774-2816)packages/feeds-client/src/feed/feed.ts (2)
currentState(274-276)FeedState(74-151)
packages/feeds-client/src/feeds-client/feeds-client.ts (5)
packages/feeds-client/src/utils/throttling/throttled-get-batched-own-fields.ts (3)
ThrottledGetBatchedOwnFields(15-16)DEFAULT_BATCH_OWN_FIELDS_THROTTLING_INTERVAL(18-18)GetBatchedOwnFieldsThrottledCallback(10-13)packages/feeds-client/src/feed/feed.ts (3)
feed(270-272)currentState(274-276)FeedState(74-151)packages/feeds-client/src/common/Poll.ts (1)
data(106-108)packages/feeds-client/src/gen/models/index.ts (1)
FeedResponse(2774-2816)packages/feeds-client/src/utils/check-own-fields-equality.ts (4)
isOwnCapabilitiesEqual(49-57)isOwnFollowsEqual(32-37)isOwnMembershipEqual(39-47)isOwnFollowingsEqual(59-67)
🔇 Additional comments (13)
packages/feeds-client/src/bindings/react/hooks/feed-state-hooks/useOwnCapabilities.ts (4)
4-6: LGTM: Imports added for new functionality.The new imports (
FeedStatefor the selector anduseFeedsClientfor feed resolution) are necessary and correctly used throughout the hook.
10-14: LGTM: Selector is optimally defined.The module-level selector definition ensures stability across renders, preventing unnecessary re-renders when used with
useStateStore. The fallback tostableEmptyArraycorrectly handles missingown_capabilities.
25-28: LGTM: Defensive defaulting ensures consistent return value.The double defaulting (
?? {}and= stableEmptyArray) ensuresfeedOwnCapabilitiesis always a defined array, even when the feed or its state is undefined. While slightly redundant, this defensive approach is appropriate for a public hook API.
19-23: Add format validation or safer parsing for feed strings containing colons.The
split(':')approach assumes the feed string follows a strict "groupId:id" format with exactly one colon. While this pattern is consistently used throughout the codebase (e.g., infeeds-client.ts:856,response-generators.ts:91, and multiple component files), the format constraint is undocumented and not validated. If eithergroupIdoridcould contain colons, this parsing would fail (e.g.,"group:id:extra"would incorrectly parse asgroupId="group"andid="id:extra").Consider using safer parsing such as
split(':').slice(0, 2)to limit to the first two parts, or adding explicit format validation.packages/feeds-client/src/utils/check-own-fields-equality.ts (1)
4-30: LGTM! Clean implementation of follow array comparison.The helper properly handles
undefinedarrays via optional chaining and uses an efficient Set-based comparison strategy. The composite key (source_feed.feed:target_feed.feed:updated_at) correctly identifies unique follow relationships.packages/feeds-client/src/feeds-client/feeds-client.test.ts (1)
408-634: Comprehensive test coverage for fieldsToUpdate logic.The new test suite thoroughly covers:
- Selective field updates based on
fieldsToUpdatearray- Preservation of non-updated fields
- Equality checks preventing unnecessary state updates
- Empty
fieldsToUpdatearray behaviorGood coverage for the new per-field update mechanism.
packages/feeds-client/src/feed/feed.test.ts (3)
217-237: Mock setup correctly extended for new throttling method.The mock type extension and setup properly includes
throttledGetBatchOwnFieldsand thefeedmethod mock withown_capabilities: undefinedstate, which aligns with the new per-feed field handling logic.
329-354: Good test for WebSocket-triggered own fields fetching.This test verifies the key behavior: when activities arrive via WebSocket and the feed hasn't been initialized (own_capabilities undefined), it triggers the batched own fields fetch. The assertion on
lastCall?.[0]correctly validates the feed argument.
457-459: Essential cleanup for test isolation.The
afterEachwithclearQueuedFeeds()ensures the queued feeds state is reset between tests, preventing cross-test contamination.packages/feeds-client/src/feed/feed.ts (2)
988-1021: Well-structured per-field update logic with WebSocket batching.The implementation correctly:
- Builds
fieldsToUpdatebased onfromWebSocketflag andenrich_own_followingsoption- Queues uninitialized feeds for batched own fields fetch when updates come from WebSocket
- Uses
own_capabilities === undefinedas the indicator for uninitialized feeds
1025-1034: Verify ifskip_own_capabilitiesandskip_own_followsshould be checked.The
shouldAddToActiveFeedsmethod currently checksskip_activity,skip_activity_current_feed, andskip_all, butskip_own_capabilitiesandskip_own_followsare defined inEnrichmentOptionsand are never checked anywhere in the codebase. Additionally, the code unconditionally fetchesown_capabilitiesandown_followsfor all newly created active feeds (lines 994–995) without respecting these skip options. However, without clear evidence of how these fields are intended to be used, it's unclear whether they should be checked here or handled elsewhere in the enrichment pipeline.packages/feeds-client/src/feeds-client/feeds-client.ts (2)
702-711: Direct state update from ownBatch response.The
ownBatchresponse now directly applies fields to feed state viapartialNext. This is a clean approach that aligns with the per-feed state management pattern.
883-929: Robust per-field equality checking and selective updates.The implementation correctly:
- Uses a mapping of field names to equality check functions
- Only includes fields that are in
fieldsToUpdateAND have changed- Applies a single
partialNextwith only the changed fieldsThis prevents unnecessary state updates and subscriber notifications. The
@ts-expect-erroron line 922 is acceptable for the dynamic field access pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/feeds-client/src/feeds-client/feeds-client.test.ts:
- Line 10: The test file imports an unused symbol 'sleep' from '../common/utils'
causing the pipeline failure; remove the unused import statement (the import of
sleep) from packages/feeds-client/src/feeds-client/feeds-client.test.ts so tests
rely on vi.useFakeTimers() and vi.advanceTimersByTimeAsync() for throttling
instead.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/feeds-client/src/feeds-client/feeds-client.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for all code in this repository
Files:
packages/feeds-client/src/feeds-client/feeds-client.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in integration-tests/ directories
Files:
packages/feeds-client/src/feeds-client/feeds-client.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Applies to **/*.test.ts : Add and extend tests with .test.ts suffix, covering FeedsClient, Feed classes, event handlers, state management, React hooks, contexts, utility functions, generated API clients, and integration tests in __integration-tests__/ directories
Applied to files:
packages/feeds-client/src/feeds-client/feeds-client.test.ts
📚 Learning: 2025-12-17T11:01:52.854Z
Learnt from: CR
Repo: GetStream/stream-feeds-js PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:01:52.854Z
Learning: Prioritize backwards compatibility, API stability, and high test coverage when changing code in this TypeScript Feeds SDK repository
Applied to files:
packages/feeds-client/src/feeds-client/feeds-client.test.ts
🧬 Code graph analysis (1)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (3)
packages/feeds-client/src/test-utils/response-generators.ts (3)
generateFollowResponse(117-133)generateFeedResponse(84-115)generateFeedMemberResponse(251-263)packages/feeds-client/src/common/Poll.ts (1)
data(106-108)packages/feeds-client/src/feed/feed.ts (1)
feed(270-272)
🪛 GitHub Actions: Lint and test
packages/feeds-client/src/feeds-client/feeds-client.test.ts
[warning] 10-10: sleep is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
[error] 10-10: unused-imports/no-unused-imports: 'sleep' is defined but never used.
🪛 GitHub Check: lint-and-test (22.x)
packages/feeds-client/src/feeds-client/feeds-client.test.ts
[failure] 10-10:
'sleep' is defined but never used
[warning] 10-10:
'sleep' is defined but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (4)
packages/feeds-client/src/feeds-client/feeds-client.test.ts (4)
42-120: LGTM: Updated test structure for new API.The tests correctly adopt the new object-based API with
fieldsToUpdateparameter. The empty array value appropriately indicates that no own_ fields should be updated during feed state initialization.
122-382: LGTM: Comprehensive coverage of own_ field updates.Excellent expansion of tests to cover
own_followingsandown_capabilities. The tests properly validate:
- Preservation of own_ fields when
fieldsToUpdateis empty- Selective updates triggering state changes only when content actually differs
- Equality checks preventing unnecessary updates when references differ but content is identical
384-409: LGTM: Solid throttling behavior test.The test properly validates throttling of
ownBatchcalls using fake timers. The time-based assertions correctly verify that multiple calls within the throttle window result in a single API call, and advancing time triggers the second batch as expected.
411-637: LGTM: Thorough fieldsToUpdate logic validation.Excellent new test suite covering the selective field update mechanism. The tests comprehensively validate:
- Field filtering based on
fieldsToUpdatearray- Exclusion behavior when fields are not listed
- Empty array behavior (no updates)
- Equality checks preventing redundant updates even when fields are eligible
This provides strong regression protection for the new per-field update flow.
🎫 Ticket:
📑 Docs: https://github.com/GetStream/docs-content/pull/
💡 Overview
📝 Implementation notes
Summary by CodeRabbit
Refactor
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.