fix(spec-014): CodeRabbit follow-ups (PR #74 review)#75
Conversation
Three findings, all valid, all locked by new tests. 1. `UintPair` rejects negative integers. The name implied unsigned but the schema only enforced length; CodeRabbit caught that an `anchor: [-1, 0]` would slip through silently. Element type tightens to `Annotated[int, Field(ge=0)]`; new tests/codegen/test_psd_manifest.py covers anchor + position. 2. Callable discriminators in both proscenio.py and psd_manifest.py return `None` for unexpected kind/type values (was `"unknown"`). Pydantic raises a clearer `union_tag_not_found` ValidationError instead of dispatching to a non-existent variant. 3. `__main__.py` docstring no longer groups `ts` with the placeholder subcommands. `ts` documents the TS bindings emit; only `godot` and `docs` stay as placeholders. Also lifts the `build_psd_manifest_schema` import in `test_schema_roundtrip.py` to module scope (was a local import in the test body). New tests under `tests/codegen/test_psd_manifest.py` lock the PSD manifest discriminated union behaviour - polygon, mesh, and sprite_frame routes + the unknown-kind rejection + the non-negative UintPair contract. Schemas + TS bindings regenerated (negative-aware UintPair surfaces in the dumped schema as `"minimum": 0` per item). proscenio-models wheel rebuilt. Refs: PR #74 review comments r3330752267 and the outside-diff __main__.py + test_schema_roundtrip.py nits. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 21 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
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 |
Three findings, all valid, all locked by new tests. 1. `UintPair` rejects negative integers. The name implied unsigned but the schema only enforced length; CodeRabbit caught that an `anchor: [-1, 0]` would slip through silently. Element type tightens to `Annotated[int, Field(ge=0)]`; new tests/codegen/test_psd_manifest.py covers anchor + position. 2. Callable discriminators in both proscenio.py and psd_manifest.py return `None` for unexpected kind/type values (was `"unknown"`). Pydantic raises a clearer `union_tag_not_found` ValidationError instead of dispatching to a non-existent variant. 3. `__main__.py` docstring no longer groups `ts` with the placeholder subcommands. `ts` documents the TS bindings emit; only `godot` and `docs` stay as placeholders. Also lifts the `build_psd_manifest_schema` import in `test_schema_roundtrip.py` to module scope (was a local import in the test body). New tests under `tests/codegen/test_psd_manifest.py` lock the PSD manifest discriminated union behaviour - polygon, mesh, and sprite_frame routes + the unknown-kind rejection + the non-negative UintPair contract. Schemas + TS bindings regenerated (negative-aware UintPair surfaces in the dumped schema as `"minimum": 0` per item). proscenio-models wheel rebuilt. Refs: PR #74 review comments r3330752267 and the outside-diff __main__.py + test_schema_roundtrip.py nits. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
PR #74 was merged before the CodeRabbit review fix-up commit landed. This PR carries that work to main as a small follow-up.
Three findings + one nit, all locked by new tests:
New `tests/codegen/test_psd_manifest.py` (6 cases) locks polygon / mesh / sprite_frame routing + unknown-kind rejection + non-negative UintPair contract. Regenerated schemas + TS bindings; `apps/blender/wheels/proscenio_models-0.1.0-py3-none-any.whl` rebuilt.
Test plan
Refs: PR #74 review comment r3330752267 + outside-diff nits.