[testbot] Add tests for streaming.py

[svc-osmo-ci]

Summary

AI-generated tests targeting file(s) with low coverage.

Issue - None

Targets selected

• src/lib/data/storage/streaming.py

Why this file was targeted

src/lib/data/storage/streaming.py — 44.9% coverage, 54 uncovered lines.

This file owns the storage SDK's public stream_object contract — the byte-range and last-N-lines read paths that back workflow log retrieval (CLI workflow logs, the log-viewer UI, and the core service's data endpoints). Tests here would catch off-by-one and boundary regressions in _seek_last_n_lines_byte_start and _stream_response dispatch: a deque/offset mistake silently returns the wrong tail, drops lines, or refetches the whole object, and the last_n_lines >= total_lines and byte_start is None fallbacks decide between a ranged second GET and a full refetch — a change there degrades to whole-file downloads with no visible error. It is the highest ROI in today's shortlist because it is a real public entry point with 10 reverse imports and genuine encoded logic, whereas the two higher-scored files fail the value bar: mux.py has zero call sites anywhere in the repo (a regression cannot reach a user yet) and utils/notify.py is best-effort email/Slack wiring whose failures are already swallowed by design.

Generator summary

All 26 tests pass with mypy and pylint clean, and the coverage self-check reports 52/54 listed lines hit (96%).

COVERAGE REPORT
- src/lib/data/storage/streaming.py: 52/54 listed lines hit (96%) — pass
  still uncovered: lines 328, 332 (unreachable — the `case _ as unreachable:
  assert_never(...)` arm of `_stream_response`. `StreamParams.options` is a
  pydantic-validated union of exactly OffsetStream | LastNLinesStream |
  FullStream, all three of which have explicit match arms, so no fourth
  variant can be constructed through the public API.)

What the 26 tests cover, all driven through the public stream_object entry point:

• Dispatch (_stream_response): all three StreamOptions arms, asserting the exact get_object kwargs so an unranged GET can't silently replace a ranged one.
• Last-N-lines seek (_seek_last_n_lines_byte_start): the offset the second ranged GET asks for on a 5-line file (18), a line reassembled across a chunk boundary (12), and a final line with no trailing newline — these are the off-by-one/deque cases flagged as the risk.
• Full-refetch fallbacks: last_n_lines == total_lines, last_n_lines > total_lines, and an empty object all take the byte_start is None path and issue an unranged second GET.
• Output shapes: bytes, lines (decode with replace/ignore/strict, newline re-termination), and the as_io file-like path including summary-only-after-close.
• StreamSummary: size, line count (None for byte streams), retries, and error-to-failure-string mapping.

Files changed: src/lib/data/storage/tests/test_streaming.py (new) and a test_streaming osmo_py_test entry in src/lib/data/storage/tests/BUILD.

One workflow note: bazel coverage and the verifier both worked, but $RUNNER_TEMP expansion was blocked by the permission layer (Contains simple_expansion) for both echo $RUNNER_TEMP and the python … "$RUNNER_TEMP/…" form the prompt recommends. I passed the literal /home/runner/work/_temp/... path instead. The generate prompt's step 10 may need updating, since the documented command form is currently unusable.

Coverage gain on listed uncovered ranges

✅ src/lib/data/storage/streaming.py — 52/54 listed lines hit (96%)

• ✅ line 166 — 1/1 hit
• ✅ lines 183-186 — 4/4 hit
• ✅ lines 195-197 — 3/3 hit
• ✅ line 207 — 1/1 hit
• ✅ lines 209-211 — 3/3 hit
• ✅ line 213 — 1/1 hit
• ✅ lines 216-218 — 3/3 hit
• ✅ line 220 — 1/1 hit
• ✅ line 222 — 1/1 hit
• ✅ line 225 — 1/1 hit
• ✅ line 272 — 1/1 hit
• ✅ line 277 — 1/1 hit
• ✅ line 283 — 1/1 hit
• ✅ line 285 — 1/1 hit
• ✅ line 292 — 1/1 hit
• ✅ line 304 — 1/1 hit
• ✅ lines 306-307 — 2/2 hit
• ✅ lines 312-313 — 2/2 hit
• ✅ lines 320-321 — 2/2 hit
• ❌ line 328 — 0/1 hit
• ❌ line 332 — 0/1 hit
• ✅ line 376 — 1/1 hit
• ✅ lines 378-381 — 4/4 hit
• ✅ line 385 — 1/1 hit
• ✅ line 387 — 1/1 hit
• ✅ line 389 — 1/1 hit
• ✅ lines 391-396 — 6/6 hit
• ✅ line 398 — 1/1 hit
• ✅ lines 404-407 — 4/4 hit
• ✅ line 410 — 1/1 hit
• ✅ line 412 — 1/1 hit

Checklist

• I am familiar with the Contributing Guidelines
• New or existing tests cover these changes
• The documentation is up to date with these changes

---

Generated by testbot pipeline

Summary by CodeRabbit

• Tests
  • Added comprehensive coverage for streaming data retrieval, including partial reads, line-based output, retries, failures, decoding, and ranged requests.
  • Added validation for empty files, small files, chunk boundaries, summaries, and file-like outputs.
• Chores
  • Improved automated test generation guidance by incorporating test quality rules and verification requirements directly into the process.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds comprehensive stream_object tests with a Bazel target. It also updates Testbot prompt construction to append TESTBOT_RULES.md and documents that prompt assembly.

Changes

Streaming test coverage

Layer / File(s)	Summary
Streaming test harness and target src/lib/data/storage/tests/BUILD, src/lib/data/storage/tests/test_streaming.py	Adds the test_streaming Bazel target and mocked storage helpers for deterministic stream tests.
Streaming behavior coverage src/lib/data/storage/tests/test_streaming.py	Tests byte, line, last-N-lines, and file-like streaming, including retries, decoding, ranges, summaries, and failures.

Testbot prompt assembly

Layer / File(s)	Summary
Testbot rules prompt wiring .github/workflows/testbot.yaml, src/scripts/testbot/TESTBOT_PROMPT.md	Appends TESTBOT_RULES.md to the Claude prompt and updates the prompt text to describe the appended rules.

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

Possibly related PRs

• NVIDIA/OSMO#1276: Contains the same Testbot prompt changes without the streaming tests.

Suggested reviewers: fernandol-nvidia, jiaenren

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding tests for the streaming.py implementation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch testbot/20260807-2334

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/data/storage/tests/test_streaming.py (1)

64-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Implement bounded reads in _FakeResumableStream.read.

When n is non-negative, return at most n bytes and preserve unread bytes for subsequent reads. Add a test for partial reads.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/data/storage/tests/test_streaming.py` around lines 64 - 69, Update
_FakeResumableStream.read to honor non-negative n by returning no more than n
bytes while retaining any unread chunk data in _remaining for later calls;
preserve draining behavior for n < 0 and keep _bytes_read accurate. Add coverage
for partial reads and subsequent retrieval of the preserved bytes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/lib/data/storage/tests/test_streaming.py`:
- Around line 64-69: Update _FakeResumableStream.read to honor non-negative n by
returning no more than n bytes while retaining any unread chunk data in
_remaining for later calls; preserve draining behavior for n < 0 and keep
_bytes_read accurate. Add coverage for partial reads and subsequent retrieval of
the preserved bytes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: af9a616d-d606-4f27-939a-7057111f4792

📥 Commits

Reviewing files that changed from the base of the PR and between ad9b898 and 277dd79.

📒 Files selected for processing (4)

• .github/workflows/testbot.yaml
• src/lib/data/storage/tests/BUILD
• src/lib/data/storage/tests/test_streaming.py
• src/scripts/testbot/TESTBOT_PROMPT.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.60%. Comparing base (ee78e47) to head (277dd79).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1277      +/-   ##
==========================================
+ Coverage   69.26%   69.60%   +0.33%     
==========================================
  Files         238      238              
  Lines       28325    28325              
  Branches     4272     4272              
==========================================
+ Hits        19620    19716      +96     
+ Misses       7865     7765     -100     
- Partials      840      844       +4     

Flag	Coverage Δ
backend	72.21% <ø> (+0.36%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.