compact_segments: replace AssertionError by warning, fixes #8535

emit only a warning, but let compaction complete.
after that, borg check --repair can fix the hints successfully.

likely this code won't be used in master branch as we only read from
legacy repos, but I ported this fix from 1.4-maint nevertheless.

This is the result of a longer discussion with Antigravity AI and me:

Detailed Explanation: Why Converting AssertionError to Warning is Correct
=========================================================================

PROBLEM OVERVIEW
----------------
The assertion `assert segments[segment] == 0` in compact_segments() was causing
borg compact to crash when segment reference counts in the hints file didn't
match the actual repository state. This typically occurred after index corruption
or repository recovery scenarios.

ROOT CAUSE ANALYSIS
-------------------
The crash happens due to a fundamental mismatch between two data structures:

1. self.segments (loaded from hints file)
   - Contains reference counts for each segment
   - Persisted to disk in the hints file
   - Represents the "last known state"

2. self.index (loaded from index file)
   - Contains mappings of object IDs to (segment, offset) pairs
   - Can be corrupted or lost
   - When corrupted, triggers auto-recovery

The Problem Scenario:
1. Repository has valid data with consistent hints.N and index.N
2. Index file gets corrupted (crash, disk error, etc.)
3. Borg detects corruption and auto-recovers:
   - Loads hints.N (with old reference counts)
   - Rebuilds index by replaying segments
   - Commits the rebuilt index
4. State is now inconsistent IF segments were deleted/lost:
   - self.segments[X] = 10 (from old hints, assumes segment X exists)
   - Segment X was actually deleted/lost
   - self.index has 0 entries for segment X (rebuilt from remaining segments)
5. During compact_segments():
   - Tries to iterate objects in segment X
   - Segment X doesn't exist (was deleted/lost)
   - OR: segment X exists but objects aren't in index (superseded)
   - segments[X] is never decremented
   - segments[X] remains 10 instead of becoming 0
   - Assertion fails!

WHY THE FIX IS CORRECT
----------------------

1. Hints are Advisory, Not Authoritative
   The hints file is an optimization to avoid scanning all segments. It's
   explicitly designed to be rebuildable from scratch by scanning segments.
   Therefore, incorrect hints should not cause a fatal error.

2. Self-Healing Behavior
   By converting the assertion to a warning and allowing compaction to proceed:
   - Compaction completes successfully
   - New hints are written with correct reference counts
   - Repository is automatically healed
   - No manual intervention required

3. Data Safety is Preserved
   The fix does NOT compromise data integrity because:
   - Compaction first copies all live data from segments to new segments
   - Only after all live data is safely copied are segments marked for deletion
   - The index determines what's "live" (authoritative source of truth)
   - Segments are deleted only when they contain no live data (per index)
   - The refcount warning indicates stale hints, not actual data loss risk
   - After compaction, new hints are written with correct counts

4. Consistent with Design Philosophy
   Borg already handles many corruption scenarios gracefully:
   - Missing hints → regenerated from segments
   - Corrupted index → rebuilt from segments
   - Missing segments → detected and handled
   This fix extends that philosophy to hint/index mismatches.

5. Alternative Solutions are Worse
   Other approaches considered:
   a) Crash and require manual intervention
      - Current behavior, user-hostile
      - Requires expert knowledge to fix
   b) Automatically run check --repair
      - Too aggressive, may hide real problems
      - User should decide when to repair
   c) Refuse to compact
      - Leaves repository in degraded state
      - Prevents normal operations

VERIFICATION
------------
The fix has been verified with test cases that reproduce both scenarios:

1. test_missing_segment_in_hints
   - Simulates missing segment files
   - Verifies compact succeeds and updates hints correctly

2. test_index_corruption_with_old_hints
   - Simulates the root cause: corrupted index with old hints
   - Verifies compact succeeds despite reference count mismatch

3. test_subtly_corrupted_hints_without_integrity
   - Existing test updated to expect warning instead of crash
   - Verifies repository remains consistent after compaction

OPERATIONAL IMPACT
------------------
After this fix:
1. Users experiencing this crash can now run `borg compact` successfully
2. The warning message alerts them to the inconsistency
3. They can optionally run `borg check --repair` for peace of mind
4. Repository continues to function normally

The warning message provides enough information for debugging while not
blocking normal operations.

CONCLUSION
----------
Converting the assertion to a warning is the correct fix because:
- It aligns with Borg's design philosophy of graceful degradation
- It enables self-healing behavior
- It preserves data safety
- It improves user experience
- It's consistent with how other corruption scenarios are handled

The assertion was overly strict for a data structure (hints) that is
explicitly designed to be advisory and rebuildable.

src/borg/legacyrepository.py

@@ -736,7 +736,12 @@ class LegacyRepository:
             for segment in unused:
                 logger.debug("complete_xfer: Deleting unused segment %d", segment)
                 count = self.segments.pop(segment)
-                assert count == 0, "Corrupted segment reference count - corrupted index or hints"
+                if count != 0:
+                    logger.warning(
+                        "Corrupted segment reference count %d (expected 0) for segment %d - corrupted index or hints",
+                        count,
+                        segment,
+                    )
                 self.io.delete_segment(segment)
                 del self.compact[segment]
             unused = []
@@ -867,7 +872,12 @@ class LegacyRepository:
                         if not self.shadow_index[key]:
                             # shadowed segments list is empty -> remove it
                             del self.shadow_index[key]
-            assert segments[segment] == 0, "Corrupted segment reference count - corrupted index or hints"
+            if segments[segment] != 0:
+                logger.warning(
+                    "Corrupted segment reference count %d (expected 0) for segment %d - corrupted index or hints",
+                    segments[segment],
+                    segment,
+                )
             unused.append(segment)
             pi.show()
             self._send_log()

src/borg/testsuite/legacyrepository_test.py

@@ -654,17 +654,20 @@ def test_subtly_corrupted_hints(repository):
         assert pdchunk(repository.get(H(2))) == b"bazz"
 
 
-def test_subtly_corrupted_hints_without_integrity(repository):
+def test_subtly_corrupted_hints_without_integrity(repository, caplog):
     make_auxiliary(repository)
     _subtly_corrupted_hints_setup(repository)
     integrity_path = os.path.join(repository.path, "integrity.5")
     os.unlink(integrity_path)
     with repository:
         repository.put(H(3), fchunk(b"1234"))
-        # do a compaction run, which fails since the corrupted refcount wasn't detected and causes an assertion failure.
-        with pytest.raises(AssertionError) as exc_info:
-            repository.commit(compact=True)
-        assert "Corrupted segment reference count" in str(exc_info.value)
+        # Do a compaction run.
+        # The corrupted refcount is detected and logged as a warning, but compaction proceeds.
+        caplog.set_level(logging.WARNING, logger="borg.legacyrepository")
+        repository.commit(compact=True)
+        assert "Corrupted segment reference count" in caplog.text
+        # We verify that the repository is still consistent.
+        assert repository.check()
 
 
 def list_indices(repo_path):