perf #345: remove dead unary AppendEntries path and stale scaffolding

[JoshuaChi]

What Does This PR Do?

Removes the dead unary handle_raft_request_in_batch path and all stale
scaffolding left behind after the hot-path migration to per-peer persistent
bidi streams landed in #333/#334.

Type:

• Bug Fix (with test)
• Feature (issue #___ approved)
• Documentation
• Test/Coverage
• Performance (with benchmark)

---

Why Is This Needed?

Closes #345.

The per-peer bidi stream replication hot path was completed in #333/#334
as part of v0.2.4. The old unary handle_raft_request_in_batch code path
was no longer on the hot path but remained in the codebase, creating ~210
lines of dead code, stale mocks, and misleading comments that no longer
reflected production behaviour.

---

Checklist

Required:

• make test passes
• Added tests for new code ← n/a (pure deletion)
• Commits squashed to 1-2 logical units

If changing APIs:

• Updated relevant docs ← n/a (internal trait, not public API)
• Explained why complexity is justified

---

Testing

How tested:

• Unit tests: cargo nextest run --all-features passes — all affected
  tests (run_success_without_joining, test_learner_bootstrap_success,
  test_merge_batch_to_write_metadata_empty_payload_with_senders) verified
• Integration tests: n/a (no behaviour change)
• Manual testing: n/a

For performance improvements:

• Bench crate compiles clean with cargo check --benches; stale
  AppendResults/PeerUpdate mock in leader_state_bench removed —
  benchmark itself unchanged, no regression possible from pure deletion

---

Does This Follow d-engine's Principles?

• Solves a real problem for most users (not just my edge case)
• Keeps implementation simple
• Doesn't bloat the API surface

---

Reviewer Notes

Pure deletion PR — no logic change, no new code. Focus areas:

1. replication/mod.rs — confirm handle_raft_request_in_batch trait
   method is fully gone and no callers remain
2. replication_handler.rs — confirm removed imports were exclusively
   used by the deleted method
3. Mock cleanup in node_test.rs and leader_state_bench.rs — confirm
   the remaining mock setup is still sufficient for existing test coverage

Estimated review complexity:

• Quick (< 100 lines)
• Medium (< 300 lines)
• Deep (> 300 lines) — but all deletions, no new logic to reason about

Summary by CodeRabbit

• Refactor
  • Restructured internal replication request batch handling to simplify the request preparation flow.
  • Updated test fixtures and mock configurations to align with the refactored replication logic.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b687a43a-75e6-4029-8cea-6259d626598c

📥 Commits

Reviewing files that changed from the base of the PR and between 741dd4f and 24a9d3f.

⛔ Files ignored due to path filters (1)

• d-engine-proto/src/generated/d_engine.client.rs is excluded by !**/generated/**

📒 Files selected for processing (7)

• d-engine-core/benches/leader_state_bench.rs
• d-engine-core/src/raft_role/leader_state_test/client_write_test.rs
• d-engine-core/src/raft_role/leader_state_test/replication_test.rs
• d-engine-core/src/replication/mod.rs
• d-engine-core/src/replication/replication_handler.rs
• d-engine-core/src/state_machine_handler/default_state_machine_handler_test.rs
• d-engine-server/src/node/node_test.rs

💤 Files with no reviewable changes (3)

• d-engine-core/src/raft_role/leader_state_test/replication_test.rs
• d-engine-core/src/replication/mod.rs
• d-engine-core/src/replication/replication_handler.rs

---

📝 Walkthrough

Walkthrough

This PR removes the handle_raft_request_in_batch method from the ReplicationCore trait and refactors ReplicationHandler to implement prepare_batch_requests instead. The old unary batch RPC path—dead code since the migration to persistent bidirectional streams—is eliminated along with its mocks, test expectations, and related imports.

Changes

Trait & Implementation Refactor

Layer / File(s)	Summary
Trait Definition d-engine-core/src/replication/mod.rs	Removed handle_raft_request_in_batch(...) method and AppendResults import from the ReplicationCore<T> trait.
Core Replication Logic d-engine-core/src/replication/replication_handler.rs	Removed handle_raft_request_in_batch implementation (batch send, response processing, quorum/peer update computation). Implemented prepare_batch_requests to generate entries locally, compute per-peer payloads, and split targets into append requests vs. snapshot targets. Updated imports to remove transport/quorum/topology-related dependencies.
Test & Mock Cleanup d-engine-core/benches/leader_state_bench.rs, d-engine-core/src/raft_role/leader_state_test/client_write_test.rs, d-engine-core/src/raft_role/leader_state_test/replication_test.rs, d-engine-core/src/state_machine_handler/default_state_machine_handler_test.rs, d-engine-server/src/node/node_test.rs	Removed mock expect_handle_raft_request_in_batch() configurations and related imports (AppendResults, PeerUpdate, HashMap). Updated test comments to reflect the simplified replication preparation flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• deventlab/d-engine#359: Updates batch-promotion flow and test mocks alongside removal of the old batch RPC entrypoint.
• deventlab/d-engine#247: Adds test expectations for handle_raft_request_in_batch—incompatible with this PR's removal of that trait method.

Poem

🐰 The batch RPC took its final bow,
No more unary streams to take a vow!
Now prepare keeps it lean and clean,
The bidirectional path reigns supreme,
Dead code vanished—long live the stream! 🌊

🚥 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 and concisely identifies the main change: removing dead unary AppendEntries path and stale scaffolding after the bidi-stream migration.
Linked Issues check	✅ Passed	All coding objectives from issue #345 are fully met: the handle_raft_request_in_batch method is removed, stale imports and mocks are cleaned up, and outdated comments are updated.
Out of Scope Changes check	✅ Passed	All changes are strictly scoped to removing the unary AppendEntries path and associated scaffolding; no unrelated modifications to core functionality or new features are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/345-append-entries-bi-stream

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!