[RudraDudhat2509]

Fixes #6278

Returns the leftover partial line once instead of continue, so the next call correctly hits read_pos == buffer size and raises StopIteration normally instead of looping forever.

Added test_line_iterator_no_trailing_newline_at_end. test_line_iterator_incomplete_line_at_end doesn't actually cover this, it only tests a stream that already ends with \n.

Verified against the pre-fix code: hangs, confirmed by killing it after 15s with no output. Full test_iterators.py 21 passed after the fix.

[RudraDudhat2509]

Gentle ping

[github-actions]

Claude finished @rsareddy0329's task in 47s —— View job

---

Code Review

• Read the PR diff and changed files
• Review LineIterator.__next__ fix for correctness
• Review the new regression test
• Post review feedback

---

I reviewed the change to sagemaker-core/src/sagemaker/core/iterators.py and the new test. The fix is correct and well-targeted. ✅

Correctness

The root cause is accurately diagnosed. In LineIterator.__next__, when the stream ends with a trailing partial line (no \n):

1. self.buffer.readline() returns the partial line, but since it doesn't end in \n, the if line and line[-1] == ord("\n") branch is skipped.
2. next(self.byte_iterator) raises StopIteration because the underlying iterator is exhausted.
3. read_pos < buffer size is True (leftover bytes remain), so the old continue re-entered the loop — but nothing had changed (read_pos, buffer, and the exhausted byte_iterator are all static), so it spun forever.

The fix returns the remainder once and advances read_pos to the buffer end. I traced the follow-up call: readline() returns b'', next(...) raises StopIteration, and now read_pos < buffer size is False (they're equal), so raise propagates a normal StopIteration. Behavior is correct and terminates. 👍

Returning the unterminated remainder (rather than dropping it) is the right call — it surfaces the final bytes to the consumer instead of silently discarding data, and it's consistent with the normal path since there is no \n to strip.

Tests

• The new test_line_iterator_no_trailing_newline_at_end correctly covers the regressed path (a chunk with no trailing \n) and asserts both that the remainder is returned and that StopIteration follows.
• The author's observation is accurate: the pre-existing test_line_iterator_incomplete_line_at_end (lines 221–233) uses a stream that does end with \n, so despite its name it never exercised this case.

Minor observation (non-blocking)

At iterators.py:184-185, remainder is re-read via seek(self.read_pos) + read(), but the line already produced by readline() at line 171 holds the same bytes — return line would avoid the extra seek/read. This is purely cosmetic; the current form is harmless and arguably more explicit, so no change is required.

No security, performance, or backward-compatibility concerns. This is a solid bug fix.

[RudraDudhat2509]

@rsareddy0329 thanks for kicking off the review. happy to fix the non blocking issue that claude pointed out, also the ci fails are unrelated to the files staged, thx again

[rsareddy0329]

@rsareddy0329 thanks for kicking off the review. happy to fix the non blocking issue that claude pointed out, also the ci fails are unrelated to the files staged, thx again

PR changes looks good and approving as claude recommendation is non-blocking.
You can include the claude recommended changes in a seperate PR.

[RudraDudhat2509]

Appreciate the quick review cycle thanks !🚀