[dajiaohuang]

Fixes #3193.

Invalid-image/segment retries now try each dataset index at most once per request. The known successful sample is still preferred, but if it has become invalid, retry continues through the remaining rows. Complete exhaustion raises a clear RuntimeError, and pipeline None is rejection rather than success. Empty/out-of-range requests raise IndexError; valid negative indices are normalized.

Tests normally import the full deeplake.integrations.mmseg.mmseg_dataset_ source module, using actual PyTorch/NumPy and source-tree exception/array helpers. Unavailable MMSeg/MMCV/prettytable and the native package bootstrap are explicitly isolated. This verifies the real wrapper's behavior without claiming a full framework training run.

Validation:

• Historical 10-case focused pytest run passed, but its segment fixture incorrectly raised InvalidImageError. Commit 4804c3f corrects that fixture to raise the actual InvalidSegmentError.
• The historical previous-head comparison failed 7 cases and passed 3; that comparison did not establish independent InvalidSegmentError coverage.
• Targeted segment mutation control: the old fixture missed removal of InvalidSegmentError handling (pass); the corrected fixture detects that removal (fails with InvalidSegmentError); the corrected fixture passes against unchanged production code. Actual PyTorch/NumPy/source exceptions were used with the same unavailable-framework isolation; this is not full SDK/framework acceptance.
• Black check and git diff --check pass.
• Disabled pytest plugin auto-loading because an unrelated installed Home Assistant plugin imports Unix-only fcntl on Windows; no target plugin was bypassed for these focused tests.
• Full native Deep Lake/MMSeg/MMCV integration and the full repository suite were not run locally.

[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: 3fbdd40a-c46a-4ac6-a46c-f995c3f01b41

📥 Commits

Reviewing files that changed from the base of the PR and between f432041 and 5ce811d.

📒 Files selected for processing (2)

• python/deeplake/integrations/mmseg/mmseg_dataset_.py
• python/tests/test_mmseg_invalid_sample_loop.py

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

📝 Walkthrough

MMSegTorchDataset.__getitem__ now advances past invalid samples before its first successful read. A regression test verifies that it reaches a valid sample after failures at indices 0 and 1.

Changes: MMSeg invalid-sample retry

Layer / File(s)	Summary
Advance past invalid samples and verify retry behavior python/deeplake/integrations/mmseg/mmseg_dataset_.py, python/tests/test_mmseg_invalid_sample_loop.py	Before any successful read, the wrapper advances to the next index when a sample is invalid. The test checks that reads at indices 0 and 1 fail, then index 2 is returned. After a success, the handler no longer updates the last successful index before retrying.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5ce81

The reported invalid-sample sequence is covered. Behavior when no valid sample remains is still uncertain, but no confirmed merge-blocking failure is established.

Security architecture risk: 🔵 Low · up to 5ce81

The change fixes recovery across consecutive invalid samples without changing access permissions or the returned data format. Production behavior when no valid sample remains is unresolved. Endless retry and stale-fallback behavior already existed, so they are not established as newly introduced vulnerabilities.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The supported exposure is dataset-read and transformation work performed by an MMSeg loading process or worker. The evidence does not establish tenant crossing, privilege escalation, or a remotely reachable attack path.

Trust Boundaries and Controls

• observed — The production transform converts every exception during image-field retrieval into InvalidImageError, which the wrapper retries. Consequently, the distinction between invalid input and terminal dataset errors depends on where the underlying access fails.

Resilience and Maintainability Implications

• inferred — If exhaustion occurs during image-field retrieval and is translated into a retried error, head can continue requesting increasing indices beyond the dataset length. Base already permitted endless retries, but repeated an unverified index rather than advancing indefinitely. Missing compiled-binding behavior prevents establishing whether this changed request pattern materially worsens production exposure.

Hardening Proposals

• proposed — Define bounded recovery and explicit terminal outcomes for exhausted candidates and repeatedly invalid fallback samples, rather than relying solely on underlying exception placement. This would harden the existing retry lifecycle; it is not a verified PR-introduced vulnerability.

Pre-merge checks: 3 · 1 · 1

Failed checks: 1 warning · 1 inconclusive

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 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check		The change satisfies the reported initial-invalid case in [#3193]. MMSegTorchDataset.__getitem__ advances from the next index while last_successful_index == -1, and the added test verifies reads `…	Confirm the behavior when all dataset samples are invalid, including the terminal exception after the index reaches the dataset boundary.

Passed checks: 3 passed

Check name	Status	Explanation
Out of Scope Changes check		The pull request changes only MMSeg invalid-sample retry handling and adds a focused regression test for issue [#3193]. The test dependency stubs support that test. No unrelated change is shown.
Title check		The title clearly and concisely describes the main change: advancing past invalid MMSeg samples.
Description check		The description explains the behavior change, references the linked issue, documents test coverage and limitations, and records validation results. It does not use every template heading or mark the i…

Full details: Linked Issues check

Explanation

The change satisfies the reported initial-invalid case in [#3193]. MMSegTorchDataset.__getitem__ advances from the next index while last_successful_index == -1, and the added test verifies reads [0, 1, 2] and returns index 2. The source has no explicit exhaustion check, and the test does not cover a dataset where every remaining sample is invalid. Therefore, the required terminal behavior after exhaustion cannot be established.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

Autopilot is currently an internal CodeRabbit preview.

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 sample line,
At zero, then one, both decline.
It hops to two and finds a view,
A valid sample, crisp and new.
The retry path now moves along,
While carrots cheer its testing song.}

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

[khustup2]

The retry logic reads correctly. Before merge:

1. Give a reproduction: a dataset whose first sample is invalid, and the old code looping forever.
2. What happens when every sample is invalid? With the new code idx grows past len(self) and the next access raises IndexError inside the loop. Bound the loop and raise a clear error.
3. Replace the ast/exec test with a test that imports the module normally.

[dajiaohuang]

Bounded retries to one attempt per dataset row, retained preference for a known successful fallback, and continued when that fallback also becomes invalid. All-invalid exhaustion now raises a clear RuntimeError; pipeline None cannot be remembered as success.

The regression tests now normally import the complete source module. All 10 focused cases pass, including actual source exception types for invalid images and segments. The previous head fails 7 of these cases. Black and diff checks pass. Actual PyTorch/NumPy and source helpers are used; unavailable MMSeg/MMCV/native bootstrap dependencies are isolated, so a full training integration pass is not claimed.

[khustup2]

All three points are addressed: retries are bounded to one attempt per row, exhaustion raises a clear RuntimeError, out-of-range input stays an IndexError, and the tests import the real module with the unavailable MM dependencies stubbed. Treating a pipeline None as a rejection matches mmdet's own CustomDataset convention.