[thweetkomputer]

Context

The Stream concurrency extent case fails intermittently with XRANGE: connection closed before response completed (CI failure). Its initialization and recovery server fixtures delete their logs during exception unwinding, leaving no server evidence in the uploaded results.

Behavior before and after

All three server phases in PrivatePlanYieldsWorkerButRetainsKeyAndRecovers now preserve logs on failure, including uncaught exceptions. Successful phases still clean up their artifacts. The existing CI upload patterns collect the retained logs.

Implementation

Enable PreserveOnFailure() on the initialization and recovery server fixtures; the mutation fixture already enables it.

Design decisions and alternatives

Reuse the existing fixture cleanup policy and CI artifact upload rules. No global fixture or workflow change is needed.

Documentation and comments

Add a comment explaining why recovery logs must survive a reply exception before Wait(). No architectural claim changes.

Test plan

• git diff --check — passed.
• pre-commit run --files tests/compact_collection_write_e2e_test.cpp — passed; unrelated hooks skipped.
• No rebuild or runtime test rerun for this log-retention-only change.

Risk assessment

Low: changes failed-test cleanup only; production behavior and successful-test cleanup are unchanged.

Rollback plan

Revert this commit to restore the previous cleanup behavior.

Reviewer guide

Review the two added PreserveOnFailure() calls and the recovery comment in tests/compact_collection_write_e2e_test.cpp.

Follow-up work

Use the retained server logs to diagnose the next Stream extent failure. This change does not fix the underlying connection closure.

Summary by CodeRabbit

• Tests
  • Improved end-to-end coverage for grouped stream write recovery.
  • Ensured server state is preserved during setup and recovery, including scenarios where a reply exception occurs before recovery logging.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6c26043a-8d3c-4632-bf0e-8d4424a6035e

📥 Commits

Reviewing files that changed from the base of the PR and between bd78f9f and 94ceda0.

📒 Files selected for processing: 1

• tests/compact_collection_write_e2e_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The end-to-end test now preserves server failure state during initial setup and recovery. A comment notes that reply exceptions can occur before Wait() reports the recovery log.

Changes

Recovery test

Layer / File(s)	Summary
Failure-state preservation tests/compact_collection_write_e2e_test.cpp	The test enables failure-state preservation during initial setup and recovery. A comment notes that reply exceptions can occur before Wait() reports the recovery log.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 94ced

No actionable merge-blocking risk remains in this test-only change.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the test change: preserving Stream concurrency failure logs. It is concise and directly related to the pull request.
Description check		The description covers all template sections. It explains the failure context, behavior change, implementation, design decisions, documentation impact, test results, risks, rollback, reviewer guidance…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description covers all template sections. It explains the failure context, behavior change, implementation, design decisions, documentation impact, test results, risks, rollback, reviewer guidance, and follow-up work.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

_{A rabbit checks the server’s state
Through setup, it keeps failures straight
Recovery holds the same safe ground
A reply may fail before logs are found
Then hops away, with tests more sound}

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