fix(persistence): treat thread reply pagination limit of 0 as unset

[VelikovPetar]

Summary

MessageDao.getThreadMessagesByParentId recently moved offline thread-reply pagination into SQL. That change turned a PaginationParams.limit of 0 into a SQL LIMIT 0 (zero rows), whereas the pre-SQL implementation treated 0 as "unlimited" and returned every reply.

Across the stack, limit: 0 means unset, not "empty":

• Backend getReplies: limit == 0 is treated as unset → default page of 100.
• iOS (Core Data): fetchLimit == 0 → unlimited (all rows).
• Flutter (pre-port): 0 → returned all rows.

This PR only applies the SQL LIMIT when a positive limit is provided, restoring the "0 = unset → return all matching replies" contract.

if (options != null && options.limit > 0) {
  query.limit(options.limit);
}

Low-risk in practice — PaginationParams.limit is non-nullable and defaults to 10, and no in-repo call site passes 0 — so this is a latent-correctness fix.

Scope is intentionally threads only. getMessagesByCid has the same unguarded pattern but is left untouched here.

Originally surfaced while reviewing #2750 (the v9 backport), where the same fix was applied.

Test

Added limit of 0 is treated as unset and returns all replies to the getThreadMessagesByParentId group: seeds 30 replies, queries with PaginationParams(limit: 0), asserts all 30 come back in ASC order. Full group passes locally.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed threaded message retrieval when pagination limit is set to 0. All matching replies are now returned in ascending order instead of none.

• Tests

  • Added coverage to verify that a zero limit returns the complete set of thread replies.

[coderabbitai]

📝 Walkthrough

Walkthrough

MessageDao.getThreadMessagesByParentId now treats PaginationParams.limit == 0 as unset, returning all matching replies. A DAO test verifies the result and ordering, and the changelog documents the fix.

Changes

Thread pagination

Layer / File(s)	Summary
Treat zero limit as unset packages/stream_chat_persistence/lib/src/dao/message_dao.dart, packages/stream_chat_persistence/test/src/dao/message_dao_test.dart, packages/stream_chat_persistence/CHANGELOG.md	SQL LIMIT is applied only for positive limits; a new test verifies that zero returns all 30 replies in ascending order, and the changelog records the behavior.

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

Possibly related PRs

• GetStream/stream-chat-flutter#2688: Changes pagination and SQL LIMIT handling in the same DAO method.

Suggested reviewers: renefloor, xsahil03x

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: thread reply pagination now treats a limit of 0 as unset.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/thread-replies-limit-zero

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/stream_chat_persistence/lib/src/dao/message_dao.dart (1)

377-378: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Same limit: 0 bug exists in getMessagesByCid.

getThreadMessagesByParentId now guards against limit: 0, but getMessagesByCid at lines 377-378 still applies query.limit(messagePagination.limit) unconditionally when messagePagination != null. If a caller passes PaginationParams(limit: 0), this generates LIMIT 0 and returns no messages — the exact bug this PR fixes for thread replies.

Consider applying the same limit > 0 guard here for consistency, or document why the behavior should differ between the two methods.

🔧 Suggested fix for consistency

-    if (messagePagination != null) {
+    if (messagePagination != null && messagePagination.limit > 0) {
       query.limit(messagePagination.limit);
     }

🤖 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 `@packages/stream_chat_persistence/lib/src/dao/message_dao.dart` around lines
377 - 378, getMessagesByCid still applies a zero pagination limit and can return
no messages. In getMessagesByCid, only call query.limit when
messagePagination.limit is greater than zero, matching the guard used by
getThreadMessagesByParentId.

🤖 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.

Outside diff comments:
In `@packages/stream_chat_persistence/lib/src/dao/message_dao.dart`:
- Around line 377-378: getMessagesByCid still applies a zero pagination limit
and can return no messages. In getMessagesByCid, only call query.limit when
messagePagination.limit is greater than zero, matching the guard used by
getThreadMessagesByParentId.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a271cfa2-91ce-4050-b962-6d99d332248c

📥 Commits

Reviewing files that changed from the base of the PR and between aab5029 and 513fa68.

📒 Files selected for processing (3)

• packages/stream_chat_persistence/CHANGELOG.md
• packages/stream_chat_persistence/lib/src/dao/message_dao.dart
• packages/stream_chat_persistence/test/src/dao/message_dao_test.dart

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.08%. Comparing base (aab5029) to head (513fa68).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2803   +/-   ##
=======================================
  Coverage   71.08%   71.08%           
=======================================
  Files         429      429           
  Lines       26891    26892    +1     
=======================================
+ Hits        19116    19117    +1     
  Misses       7775     7775           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.