fix: avoid reversing chat history messages

[steveb05]

This fixes an issue where chat messages were being sent in reverse order during partial blocking resends, causing Minecraft clients to disconnect. The problem was in the resendMessages method which was using reversed().take() instead of takeLast() to get recent messages, resulting in messages being delivered with incorrect sequential ordering.

Summary by CodeRabbit

• Bug Fixes
  • Resent chat messages during partial blocking now appear in chronological order (oldest to newest) for a natural reading flow.
  • This replaces the previous newest-first delivery that could be confusing when catching up on recent messages.
  • No changes to features, settings, or permissions—only the display order of resent messages was refined.

[coderabbitai]

Walkthrough

Adjusted resend order in ChatHistoryHandler for PartialBlocking: last N messages are selected with takeLast(...) and sent in chronological (oldest-to-newest) order. No other control flow or signatures changed; status is reset to PartialBlocking(0).

Changes

Cohort / File(s)	Summary of Changes
Chat resend order update engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt	In the PartialBlocking branch of resendMessages, replaced messages.reversed().take(status.newMessages) with messages.toList().takeLast(status.newMessages) so resent messages are delivered oldest→newest. Final state still set to PartialBlocking(0); no signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller as Caller
  participant CH as ChatHistoryHandler
  participant Store as MessagesStore
  participant Client as PlayerClient

  Caller->>CH: resendMessages(status=PartialBlocking(N))
  CH->>Store: retrieve messages
  Note over CH,Store: Select last N via takeLast(N)
  CH->>Client: send messages chronological (oldest → newest)
  CH->>CH: set status to PartialBlocking(0)

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 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 change by stating that the fix prevents reversing chat history messages, which aligns with the PR’s focus on correcting message order during resends. It is concise, specific, and free of extraneous details.

Poem

I sift through chats with twitching ear,
Last N crumbs of text appear—
Not newest-first, but time aligned,
A tidy trail the bytes refined.
Thump-thump goes QA’s gentle paw—hip hop, hurrah! 🐇✨

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4f57a and 04c9891.

📒 Files selected for processing (1)

• engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt (2)

258-260: Avoid allocation when there are no new messages.

Short-circuit to skip the toList() copy when newMessages == 0.

Apply this diff:

-                messages.toList().takeLast(status.newMessages).forEach { it.send(player) }
+                if (status.newMessages > 0) {
+                    messages.toList().takeLast(status.newMessages).forEach { it.send(player) }
+                }

---

258-260: Add a regression test for resend ordering.

Recommend a test that sets PartialBlocking, appends messages A,B,C, triggers resendMessages, and asserts delivery order A→B→C for the last N.

I can draft the test scaffold if you want.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ab530e and 9d4f57a.

📒 Files selected for processing (1)

• engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt (1 hunks)

⏰ 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: build

🔇 Additional comments (2)

engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/interaction/ChatHistoryHandler.kt (2)

258-260: Correct fix: resend last N in chronological order (no reversal).

Using toList().takeLast(status.newMessages) preserves order and avoids the previous newest-first bug. Matches the PR goal and should prevent client disconnects tied to out-of-order resends.

---

258-260: No additional .reversed().take(...) usages found.

A recursive grep across all .kt files returned no matches for .reversed().take(. No further occurrences exist—no action needed.