feat(streaming): rebuild bounded result records on current main - #172
Conversation
📝 WalkthroughWalkthrough
Changes배치 결과 스트리밍
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🟡 Moderate · up to The new streaming client performs credential and provider access without an explicitly validated tenant scope, which could weaken tenant isolation or use an unintended scope. Merge should wait for scope validation and the related contract documentation. Sequence Diagram(s)sequenceDiagram
participant Caller
participant StreamingBatchAPIClient
participant BatchAPIClient
participant ProviderAPI
Caller->>StreamingBatchAPIClient: iter_batch_records 호출
StreamingBatchAPIClient->>BatchAPIClient: 배치 상태 조회
BatchAPIClient->>ProviderAPI: 결과 파일 스트리밍 요청
ProviderAPI-->>StreamingBatchAPIClient: JSONL 바이트 청크
StreamingBatchAPIClient-->>Caller: BatchResultRecord 반환
StreamingBatchAPIClient->>ProviderAPI: 오류 파일 스트리밍 요청
ProviderAPI-->>StreamingBatchAPIClient: JSONL 바이트 청크
StreamingBatchAPIClient-->>Caller: 오류 BatchResultRecord 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pg_llm_batch/result_streaming.py (1)
61-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value제한 검증 방식을 기존 헬퍼와 통일하십시오.
pg_llm_batch/token_counter.py의_require_positive_limit은type(value) is not int로 정확한 int만 허용합니다. 여기서는isinstance를 사용하므로IntEnum같은 int 서브클래스가 통과합니다. 동작 차이는 작지만, 두 검증 경로의 규칙을 같게 유지하면 계약이 명확해집니다.🤖 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 `@pg_llm_batch/result_streaming.py` around lines 61 - 69, Update _validate_positive_integer to use the same exact-int check as token_counter.py’s _require_positive_limit, rejecting int subclasses such as IntEnum while preserving the existing positive-value validation and ValidationError behavior.pg_llm_batch/__init__.py (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value공개 API 문서에
BatchResultRecord도 추가하십시오.
__all__은BatchResultRecord와StreamingBatchAPIClient를 함께 노출합니다. 그러나 모듈 docstring의 API 목록에는StreamingBatchAPIClient만 있습니다. 두 이름을 모두 나열하면 문서와 공개 계약이 일치합니다.♻️ 제안 변경
StreamingBatchAPIClient -- bounded incremental result records + BatchResultRecord -- immutable streamed result/error recordAlso applies to: 43-51
🤖 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 `@pg_llm_batch/__init__.py` at line 9, 모듈 docstring의 공개 API 목록에 __all__로 노출되는 BatchResultRecord를 추가하고, 기존 StreamingBatchAPIClient 항목은 유지하여 문서와 실제 공개 계약이 일치하도록 수정하세요.
🤖 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.
Inline comments:
In `@pg_llm_batch/result_streaming.py`:
- Around line 72-74: Ensure all parsed JSON floating-point values are finite,
including exponent-overflow values such as 1e999: add a parse_float hook
alongside _reject_non_finite_json_constant that validates the converted value
and raises ValueError for non-finite results, then wire it into the relevant
JSON parsing calls. Add coverage for b'{"value":1e999}\n' and preserve rejection
of NaN and Infinity literals.
---
Nitpick comments:
In `@pg_llm_batch/__init__.py`:
- Line 9: 모듈 docstring의 공개 API 목록에 __all__로 노출되는 BatchResultRecord를 추가하고, 기존
StreamingBatchAPIClient 항목은 유지하여 문서와 실제 공개 계약이 일치하도록 수정하세요.
In `@pg_llm_batch/result_streaming.py`:
- Around line 61-69: Update _validate_positive_integer to use the same exact-int
check as token_counter.py’s _require_positive_limit, rejecting int subclasses
such as IntEnum while preserving the existing positive-value validation and
ValidationError behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c1e7bc08-3039-4414-ba2a-3f68c968abd7
📒 Files selected for processing (6)
pg_llm_batch/__init__.pypg_llm_batch/result_streaming.pytests/test_bounded_jsonl_physical_line_budget.pytests/test_bounded_jsonl_result_streaming.pytests/test_bounded_jsonl_result_streaming_coverage.pytests/test_streaming_transport_handoff.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pg_llm_batch/result_streaming.py (1)
107-115: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
tenant_scope를 명시적으로 받고 provider 경계 전에 검증하십시오.
StreamingBatchAPIClient는get_batch_status()와_iter_jsonl_file()에서 자격 증명 조회와 provider I/O를 수행합니다. 생성 시 검증된 tenant scope를 받지 않으므로 이 경계 전에 scope를 검증할 수 없습니다.
tenant_scope를 추가하고 기본값은 정확히"standalone"로 유지하십시오.endpoint_alias, remote ID, provider 데이터 또는 transport header에서 scope를 도출하지 마십시오. 계약 변경에 맞춰 관련 문서와 CHANGELOG를 갱신하십시오.DurableBatchAPIClient의 기존 네 인수 recorder seam은 유지하십시오.🤖 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 `@pg_llm_batch/result_streaming.py` around lines 107 - 115, StreamingBatchAPIClient.__init__에 tenant_scope 인수를 추가하고 기본값을 정확히 "standalone"으로 설정한 뒤, get_batch_status()와 _iter_jsonl_file()이 자격 증명 조회 또는 provider I/O 전에 생성 시 전달된 scope를 검증하도록 연결하십시오. scope는 endpoint_alias, remote ID, provider 데이터, transport header에서 추론하지 말고, 관련 문서와 CHANGELOG를 계약 변경에 맞게 갱신하며 DurableBatchAPIClient의 기존 네 인수 recorder seam은 유지하십시오.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@pg_llm_batch/result_streaming.py`:
- Around line 107-115: StreamingBatchAPIClient.__init__에 tenant_scope 인수를 추가하고
기본값을 정확히 "standalone"으로 설정한 뒤, get_batch_status()와 _iter_jsonl_file()이 자격 증명 조회
또는 provider I/O 전에 생성 시 전달된 scope를 검증하도록 연결하십시오. scope는 endpoint_alias, remote
ID, provider 데이터, transport header에서 추론하지 말고, 관련 문서와 CHANGELOG를 계약 변경에 맞게 갱신하며
DurableBatchAPIClient의 기존 네 인수 recorder seam은 유지하십시오.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47c87992-b33c-49df-a0c2-21b56c28eb59
📒 Files selected for processing (3)
pg_llm_batch/__init__.pypg_llm_batch/result_streaming.pytests/test_result_streaming_numeric_contract.py
🚧 Files skipped from review as they are similar to previous changes (1)
- pg_llm_batch/init.py
1 participant
Current-main replacement for #58
This PR rebuilds the bounded result-streaming slice from protected main rather than transferring #58's stale stack ancestry. No #58 checks, reviews, approvals, generated-merge evidence, or stale-base evidence transfer.
Test-first reconstruction
86748e96968725509b343d67e09dbd6b42b1fc8freplayed realistic incremental result/error JSONL regressions and failed because the protected package had noBatchResultRecord/StreamingBatchAPIClientsurface.BatchAPIClientcredential, timeout, retry, response-size, retention, lifecycle, and session-ownership contracts.ValidationErrorevidence and the protected post-response-handoff no-replay contract; those were aligned without weakening production behavior.ac8788c2e5d8e78b17c6ad6539505260218361d1then found one genuine acceptance defect: the finite-return path of_reject_non_finite_float()was uncovered even though the exponent-overflow rejection path was covered.d163245a1cae534a5e1b574ae51716594ad1d2aeadds the narrow realistic finite JSON-float regression (1.25) so both finite acceptance and non-finite rejection are exercised.The feature provides incremental result/error record parsing, strict UTF-8/JSON-object framing, duplicate-member and non-finite-number rejection, per-line/record/physical-line/download ceilings, batch-wide physical-line accounting, deterministic early close, and a no-replay boundary once a provider response has been handed to the consumer. It adds no schema, migration, credential, model, scheduler, package-version, or release authority.
Exact-head and live-base evidence
Protected main is
c262d9d60e559855ac943a53f6ae084f406c6a40, and this PR remains mergeable against it. On current exact headd163245a1cae534a5e1b574ae51716594ad1d2ae, repository CI is GREEN including Python 3.10/3.12/3.14, exact owned production coverage/docstrings/package, lock freshness, and component/PostgreSQL container smokes; Release Acceptance is GREEN. Current exact-head CodeQL, OSV, Trivy, Semgrep OSS, and OpenCode evidence is also terminal-success.The current exact-head Strix required workflow is still in progress, so this PR is not yet merge-eligible. Pending required evidence is not transferred from the predecessor source head. Final review-thread state and every live required workflow must be freshly rechecked after Strix becomes terminal.
Merge boundary
Merge only while exact head
d163245a1cae534a5e1b574ae51716594ad1d2aeremains unchanged and mergeable against the then-live protected base, all live required workflows are terminal-success, and zero valid unresolved review findings remain. After protected integration, close #58 only after fresh comparison proves its unique bounded-streaming behavior is preserved, then rebuild #59 and downstream checkpoint/audit work from the protected result in dependency order.Summary by CodeRabbit
새로운 기능
버그 수정