[rixnobis]

Squaresoft74's -v log of the failure shows a host byte lost on the way in: the monitor answers the 0x000f8000 chunk with ECKSUM only once the next chunk's first byte has arrived, 0x000fa000 gets no reply at all, and 0x000fc000 and 0x000fe000 are ACKed. psxmon stopped at the first ECKSUM.

It now PINGs back into step and resends from the failed chunk (an LZ4 block from offset 0), up to three times. ECKSUM, EBADLEN, a damaged reply and a missing reply count as link damage; any other monitor error still ends the transfer. The sim can now drop a host byte, and the new tests fail on main with the reported LOAD at ... failed: ECKSUM (0x06).

Fixes #42.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8e63faf4-be3e-44e1-913d-43962341b0ab

📥 Commits

Reviewing files that changed from the base of the PR and between 6457971 and 43b7409.

📒 Files selected for processing: 3

• src/session.rs
• tests/session/main.rs
• tests/session/sim.rs

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

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Bulk memory loads and writes now retry up to three times after link-damage errors, resynchronizing before each retry.
  • Uncompressed transfers resume from the first unacknowledged chunk, preserving completed progress. Compressed transfers restart from the beginning.
  • Transfers stop retrying when the retry limit is reached or recovery is unsuccessful, and continue to report errors for other failure types.

Bulk LOAD and WRITE_MEM transfers now retry selected link-damage failures after PING resynchronization. Raw transfers resume from the first unacknowledged chunk, while LZ4 transfers restart from the beginning. Simulator support and tests cover dropped host bytes and retry limits.

Changes: Bulk transfer recovery

Layer / File(s)	Summary
Retry and resume transfer data src/session.rs	The session retries link-damage failures up to three times after PING resynchronization. Raw transfers continue from the first unacknowledged chunk. LZ4 transfers restart from the beginning.
Simulate dropped bytes and verify recovery tests/session/sim.rs, tests/session/main.rs	The simulator can discard configured host bytes. Tests cover recovery for pipelined plain and LZ4 loads, persistent checksum failures, and pipelined WRITE_MEM errors.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Merge Risk: ⚪ Minimal · up to 43b74

No merge-blocking issue was identified; the transfer-recovery change is ready for normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 72.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 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 main change: retrying LOAD or WRITE_MEM transfers after link damage.
Description check		The description directly explains the link-damage scenario, retry behavior, error handling, simulator changes, and tests.
Linked Issues check		Issue #42 reports intermittent LOAD checksum failures but defines no specific coding acceptance criteria. The PR adds retry and PING resynchronization for LOAD and WRITE_MEM link damage, restarts LZ4 …
Out of Scope Changes check		The changes stay within issue #42. The session changes implement recovery from damaged bulk transfers. The simulator changes inject the reported link fault. The tests verify recovery and retry-limit b…

Full details: Linked Issues check

Explanation

Issue #42 reports intermittent LOAD checksum failures but defines no specific coding acceptance criteria. The PR adds retry and PING resynchronization for LOAD and WRITE_MEM link damage, restarts LZ4 transfers from offset zero, and limits retries to three. The PR adds simulator support for dropped host bytes and tests for plain-load recovery, LZ4 recovery, and persistent ECKSUM failure.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #42. The session changes implement recovery from damaged bulk transfers. The simulator changes inject the reported link fault. The tests verify recovery and retry-limit behavior. No unrelated product behavior is shown in the supplied change summary.

✨ 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

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