fix(models): reject non-string BatchRequest fields - #104
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesBatchRequest 경계 동작
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/doctoring/batch-request-representation-confidentiality.md`:
- Around line 21-24: The Compatibility section’s statement that accepted values
are unchanged is inaccurate. Update the documentation to state that the
constructor signature is unchanged but runtime accepted values are narrowed:
user_prompt, model, and id require exact str values, while system_prompt accepts
only None or exact str; preserve the existing notes about equality and
interfaces.
In `@tests/test_batch_request_type_boundary.py`:
- Around line 10-29: Extend
test_batch_request_rejects_non_string_optional_and_identifier_fields and the
related BatchRequest validation tests to pass a custom str subclass for each of
user_prompt, model, system_prompt, and id, asserting ValidationError and the
corresponding field detail; ensure the tests require exact string types rather
than accepting isinstance(value, str).
🪄 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: bb0b545f-f4ce-4ad8-8bab-0ed722af2249
📒 Files selected for processing (3)
docs/doctoring/batch-request-representation-confidentiality.mdpg_llm_batch/models.pytests/test_batch_request_type_boundary.py
1 participant
RCA
BatchRequestis a public package-root data class, but protectedmainrelied on annotations rather than runtime validation. False-valued non-string prompts such as integer0could therefore crossTokenCounter.count_tokens()as zero-token text, while non-string model/system/id values could reach unrelated lower-layer failures. The same original validation repair initially copied rejected caller values intoValidationError.value, which could disclose prompt/model/id content through exception rendering.A later representation audit found a second privacy boundary in this same value object: the generated dataclass
repr()exposeduser_prompt,system_prompt, and caller-selectedid. Generic debugging/assertion/logging surfaces should not become package-owned copies of request content.Bounded implementation
user_prompt,model, andidmust be exact strings;system_promptmust beNoneor an exact string.<redacted>validation evidence.user_prompt,system_prompt, andiduse dataclassrepr=False;modelremains visible for useful non-content diagnostics.repr=Falseis deliberately not claimed as a serialization boundary: direct attribute access,vars()/__dict__,dataclasses.asdict(), pickling, and caller-defined serializers remain separate caller-controlled surfaces.Test-first evidence
39a37f3159ee618f05629c5548529494bc257a7destablished exact runtime type failures.dd300b017e953593356a2fff9f262fc291b74e69exposed rejected-value confidentiality leakage.repr()andstr()while the model remains visible, plusdocs/doctoring/batch-request-representation-confidentiality.mdwith Python 3.14 dataclass primary documentation in APA 7 form.Predecessor checks are development provenance only.
Current exact staged state
c2be6651d7133459685ccb98cdf38e75ee624d98.main:bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.31466781421: completed / success.31466781431: completed / success.31466781388: completed / success.These PR-triggered workflows still run under protected-main pre-#88 checkout semantics, so terminal success is staged integration evidence rather than final exact contributor-head acceptance.
Integration boundary
Keep Draft while repository exact-source governance #88 and its read-only central review-evidence prerequisite remain outside protected
main. Do not churn this clean source merely to retrigger unchanged external infrastructure. After that prerequisite materially changes, reacquire every applicable exact-source CI/security/coverage/package/provenance/review/policy gate on the unchanged final source and merge only if live policy is genuinely satisfied.Summary by CodeRabbit
개선 사항
BatchRequest생성 시 주요 필드의 타입을 엄격히 검증합니다.문서