Feature/iron validation#3
Open
sake92 wants to merge 11 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR introduces a pluggable validation backend system enabling optional Iron constraint generation for OpenAPI code generation. It adds three backend implementations, integrates validation into the code generation pipeline, threads validation type mappings through model generators, and wires CLI parameters and integration tests. ChangesIron Validation Backend Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-06-05-iron-validation.md (1)
156-157: ⚡ Quick winClarify the expected compile result in Task 2 to avoid contradictory guidance.
“compile succeeds” conflicts with “backends not yet created … pending compilation errors.” Please make this expectation deterministic (either require temporary stubs first, or expect failure until Task 3/4).
🤖 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 `@docs/superpowers/plans/2026-06-05-iron-validation.md` around lines 156 - 157, Clarify Task 2's expected compile outcome by either (A) stating that compilation should succeed and instructing the author to add minimal temporary stubs for IronValidationBackend and ValidsonValidationBackend so the build passes, or (B) stating that compilation is expected to fail until Task 3 and Task 4 add the real backends; update the text to explicitly choose one of these two options and reference the backend symbols IronValidationBackend and ValidsonValidationBackend and the follow-up tasks (Task 3/Task 4) so readers know whether they must add stubs in Task 2 or await later tasks.
🤖 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/superpowers/plans/2026-06-05-iron-validation.md`:
- Around line 16-31: The Markdown file contains two adjacent tables that violate
markdownlint rule MD058; edit
docs/superpowers/plans/2026-06-05-iron-validation.md and insert a single blank
line before and after each table block (i.e., add an empty line separating the
preceding paragraph and the following content paragraph from each table) so both
tables are surrounded by blank lines and the MD058 warning is resolved.
- Around line 329-351: The two buildNumericConstraintKey methods overload only
by Option[Long] vs Option[Double], which erases to the same JVM signature;
rename one (e.g., buildNumericConstraintKeyLong and
buildNumericConstraintKeyDouble) or make a single generic method (e.g.,
buildNumericConstraintKey[T](prefix:String, minimum: Option[T], maximum:
Option[T])) and update all call sites (notably where Num32/Num64 construct keys)
to call the new names or the generic form so compilation no longer fails due to
signature erasure.
In
`@openapi4s/src/main/scala/ba/sake/openapi4s/validation/IronValidationBackend.scala`:
- Around line 105-106: The Password branch in the pattern matching (case pwd:
SchemaDefinition.Password) always returns Some(...) even when pwd.minLength,
pwd.maxLength and pwd.pattern are absent, which later causes a reduceLeft on an
empty constraints list to crash; change the branch to return None when there are
no actual constraints (minLength/maxLength are empty and pattern is empty/None)
and only return Some(buildConstraintKey(...)) when at least one constraint is
present; apply the same presence-check approach to the other
constraint-producing branches that later feed the reduceLeft so they also return
None when they produce no constraints.
- Around line 140-141: The code currently concatenates raw regexes with "|" via
pattern.foreach(p => parts += s"Match=$p") and parts.result().mkString("|"),
then later splits on "|" which corrupts alternation-containing patterns; fix by
encoding each pattern before joining (e.g. Base64 or URL-encode each p when
building the parts) and decode when parsing (replace the split-by-"\\|" logic
with decoding of each encoded segment), or alternatively switch to a delimiter
guaranteed not to appear in regexes (and update both the builder and the parser
to use that delimiter). Ensure the change updates both the construction site
(pattern.foreach / parts.result().mkString) and the parsing site that currently
splits on "|" so Match constraints with alternation remain intact.
- Around line 197-209: The code in IronValidationBackend.scala that builds Range
constraints (the Type.Apply(Type.Name("GreaterEqual")/ "LessEqual") branches)
incorrectly converts numeric bounds using value.toDouble.toInt, which truncates
Int64/Long limits; change the conversion to respect baseType: when baseType ==
"Int" use value.toInt and emit Lit.Int(...), when baseType == "Long" use
value.toLong and emit Lit.Long(...), and keep Float/Double using value.toDouble
with Lit.Double; update both the "Min=" and "Max=" branches (the
Type.Apply(Type.Name("GreaterEqual") and Type.Apply(Type.Name("LessEqual"))
sites) accordingly so Long bounds are preserved.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-06-05-iron-validation.md`:
- Around line 156-157: Clarify Task 2's expected compile outcome by either (A)
stating that compilation should succeed and instructing the author to add
minimal temporary stubs for IronValidationBackend and ValidsonValidationBackend
so the build passes, or (B) stating that compilation is expected to fail until
Task 3 and Task 4 add the real backends; update the text to explicitly choose
one of these two options and reference the backend symbols IronValidationBackend
and ValidsonValidationBackend and the follow-up tasks (Task 3/Task 4) so readers
know whether they must add stubs in Task 2 or await later tasks.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c70b2963-6e6a-4807-93c6-b9c98ce1d7cb
📒 Files selected for processing (11)
.gitignorecli/src/main/scala/ba/sake/openapi4s/cli/OpenApi4sMain.scaladocs/superpowers/plans/2026-06-05-iron-validation.mdopenapi4s/src/main/scala/ba/sake/openapi4s/ModelBackend.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/OpenApiWriter.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/ValidationBackend.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/circe/CirceModelGenerator.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/validation/IronValidationBackend.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/validation/NoneValidationBackend.scalaopenapi4s/src/main/scala/ba/sake/openapi4s/validation/ValidsonValidationBackend.scalaopenapi4s/src/test/scala/ba/sake/openapi4s/OpenApiGeneratorSuite.scala
Comment on lines
+16
to
+31
| | # | File | Purpose | | ||
| |---|------|---------| | ||
| | C1 | `openapi4s/src/main/scala/ba/sake/openapi4s/ValidationBackend.scala` | Trait + ValidationBackendId sealed hierarchy | | ||
| | C2 | `openapi4s/src/main/scala/ba/sake/openapi4s/validation/NoneValidationBackend.scala` | No-op default | | ||
| | C3 | `openapi4s/src/main/scala/ba/sake/openapi4s/validation/IronValidationBackend.scala` | 2-pass collect, dedup, generate Newtypes.scala + type map | | ||
| | C4 | `openapi4s/src/main/scala/ba/sake/openapi4s/validation/ValidsonValidationBackend.scala` | Thin wrapper delegating to ValidsonUtils | | ||
|
|
||
| ### Modify | ||
| | # | File | Change | | ||
| |---|------|--------| | ||
| | M1 | `openapi4s/src/main/scala/ba/sake/openapi4s/OpenApiWriter.scala` | Add `validation` to Config, wire ValidationBackend, call its generate | | ||
| | M2 | `openapi4s/src/main/scala/ba/sake/openapi4s/ModelBackend.scala` | Pass validation info to circe generator factory | | ||
| | M3 | `openapi4s/src/main/scala/ba/sake/openapi4s/circe/CirceModelGenerator.scala` | Accept optional `newtypeMap`, swap types, add iron.circe.given import | | ||
| | M4 | `cli/src/main/scala/ba/sake/openapi4s/cli/OpenApi4sMain.scala` | Add `--validation` arg | | ||
| | M5 | `openapi4s/src/test/scala/ba/sake/openapi4s/OpenApiGeneratorSuite.scala` | Add integration test for `--models circe --validation iron` | | ||
|
|
There was a problem hiding this comment.
Add blank lines around the two tables to satisfy markdownlint MD058.
This keeps docs CI/lint clean and avoids noisy warnings.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 16-16: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 24-24: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@docs/superpowers/plans/2026-06-05-iron-validation.md` around lines 16 - 31,
The Markdown file contains two adjacent tables that violate markdownlint rule
MD058; edit docs/superpowers/plans/2026-06-05-iron-validation.md and insert a
single blank line before and after each table block (i.e., add an empty line
separating the preceding paragraph and the following content paragraph from each
table) so both tables are surrounded by blank lines and the MD058 warning is
resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests