Skip to content

Conversation

@superg
Copy link
Owner

@superg superg commented Jan 18, 2026

Summary by CodeRabbit

  • Improvements
    • SCSI and C2 error counting now uses sample-based calculations for improved accuracy.
    • Error reporting format updated with "samples" suffix in final summaries for clarity.
    • Logging labels standardized for consistency across error types.
    • Internal state handling refined for better data consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

📝 Walkthrough

Walkthrough

Changes to error counting and logging in CD dump processing modify SCSI error increments from 1 to CD_DATA_SIZE_SAMPLES, replace C2 bit-count logic with sample counting via sector state derivation, update logging labels to "SCSIs/C2s", and append "samples" suffix to final error reporting.

Changes

Cohort / File(s) Summary
Error Counting & State Handling
cd/cd_dump.ixx
Modified SCSI error increment from 1 to CD_DATA_SIZE_SAMPLES on session gaps; replaced C2 bit-count logic with sample counting via c2_to_state() and State::ERROR_C2 occurrence counting; updated state derivation to use sector_state instead of recomputing from sector_c2; adjusted logging labels from category-specific (SCSI, C2, Q) to generic (SCSIs, C2s, Q) without conditional suffixes; changed final error summary reporting to include "samples" suffix for SCSI and C2 counts

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 We count in samples, not by one,
C2 states help us have our fun,
Logs label cleaner, sector_state so bright,
CD dumps now working just right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: converting error counting and reporting from sector-based metrics to sample-based metrics throughout the dump logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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


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

Comment @coderabbitai help to get the list of available commands and usage tips.

@superg superg merged commit e15b72e into main Jan 18, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants