fix: respect enrichment settings when update feed state from activity… #210
Conversation
📝 WalkthroughWalkthroughFieldsToUpdate construction for newly discovered feeds was changed to always include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/feeds-client/src/feed/feed.ts:
- Around line 993-1002: The change altered the push order on fieldsToUpdate
(currently pushing 'own_membership' first), breaking tests that expect the
previous order; revert to backward-compatible ordering by moving the
fieldsToUpdate.push('own_membership') so it is executed after the existing
pushes for 'own_capabilities', 'own_follows' and conditional 'own_followings'
(i.e., ensure enrichmentOptions checks for skip_own_capabilities,
skip_own_follows, and enrich_own_followings run before adding 'own_membership'),
or alternatively update all affected tests to expect the new order — but
preferred fix is to reorder the pushes in feed.ts to keep 'own_membership' last.
📜 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 (2)
packages/feeds-client/src/feed/feed.test.tspackages/feeds-client/src/feed/feed.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.tspackages/feeds-client/src/feed/feed.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/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: 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.tspackages/feeds-client/src/feed/feed.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/feed/feed.tspackages/feeds-client/src/feed/feed.test.ts
🧬 Code graph analysis (1)
packages/feeds-client/src/feed/feed.test.ts (2)
packages/feeds-client/src/feed/feed.ts (1)
feed(270-272)packages/feeds-client/src/test-utils/response-generators.ts (2)
generateFeedResponse(84-115)generateActivityResponse(135-167)
⏰ 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 (2)
packages/feeds-client/src/feed/feed.ts (1)
994-999: LGTM! Enrichment flag logic is correct.The conditional logic properly respects the
skip_own_capabilitiesandskip_own_followsflags, using safe optional chaining to handle undefined enrichmentOptions.packages/feeds-client/src/feed/feed.test.ts (1)
457-676: Excellent comprehensive test coverage for enrichment options.The test suite thoroughly covers all combinations of enrichment flags:
- Individual flags (skip_own_capabilities, skip_own_follows, enrich_own_followings)
- Boolean values (true/false)
- Multiple flag combinations
- Edge cases (both skip flags true)
The tests are well-structured, descriptive, and properly isolated. Based on learnings, high test coverage is prioritized in this repository, and this PR delivers on that expectation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/feeds-client/src/feed/feed.test.ts (1)
440-455: Consider clarifying test name to indicate default behavior.While the test is correct, the title "should always include..." might be slightly misleading given that the new tests below demonstrate these fields can be excluded via skip flags. Consider renaming to make it clear this tests the default behavior, e.g., "should include own_capabilities, own_follows, own_membership by default when not from WebSocket".
♻️ Optional test name refinement
- it('should always include own_capabilities, own_follows, own_membership when not from WebSocket', () => { + it('should include own_capabilities, own_follows, own_membership by default when not from WebSocket', () => {
📜 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/feed/feed.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/feed/feed.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/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/feed/feed.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/feed/feed.test.ts
🧬 Code graph analysis (1)
packages/feeds-client/src/feed/feed.test.ts (2)
packages/feeds-client/src/feed/feed.ts (1)
feed(270-272)packages/feeds-client/src/test-utils/response-generators.ts (2)
generateFeedResponse(84-115)generateActivityResponse(135-167)
⏰ 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 (4)
packages/feeds-client/src/feed/feed.test.ts (4)
300-437: LGTM! Expectations updated to reflect new fieldsToUpdate behavior.The test expectations have been correctly updated to include
own_membershipas the first element in thefieldsToUpdatearray. This aligns with the PR objective to ensure enrichment settings are respected when updating feed state fromactivity.current_feed.
457-509: Excellent coverage of skip_own_capabilities flag behavior.The tests properly verify that:
- When
skip_own_capabilitiesistrue, theown_capabilitiesfield is excluded fromfieldsToUpdate- When
skip_own_capabilitiesisfalse, theown_capabilitiesfield is includedTest structure is clear and assertions are precise.
511-563: Excellent coverage of skip_own_follows flag behavior.The tests properly verify that:
- When
skip_own_followsistrue, theown_followsfield is excluded fromfieldsToUpdate- When
skip_own_followsisfalse, theown_followsfield is includedConsistent with the
skip_own_capabilitiestests and provides thorough individual flag coverage.
565-676: Outstanding coverage of enrichment flag combinations!These tests comprehensively verify the interaction between multiple enrichment flags:
- Both
skip_own_capabilitiesandskip_own_followsset totruecorrectly results in onlyown_membership- Skip flags properly combine with
enrich_own_followings, showing that:
own_followingsis added when enrichment is enabled- Skip flags still control their respective fields independently
- All three meaningful combinations are tested (both skip flags, each individual skip flag with enrich_own_followings)
This demonstrates proper orthogonality of the enrichment options and ensures the implementation respects all flag combinations correctly.
….current_feed
🎫 Ticket: https://linear.app/stream/issue/REACT-739/respect-enrichment-options-when-updating-feed-from-current-feed
📑 Docs: N/A
💡 Overview
📝 Implementation notes
Summary by CodeRabbit
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.