feat(parser,validator): support bare-boolean schemas for OAS 3.1+ - #450
Merged
Conversation
JSON Schema 2020-12 allows `true` (accept anything) and `false` (accept
nothing) wherever a schema is expected, and OAS 3.1 adopted that dialect
wholesale. oastools rejected the form outright: decoding a scalar into
the Schema struct alias failed with "cannot construct !!bool into
parser.Alias", so a whole document was refused for one boolean.
Modeled additively as Schema.BoolForm *bool, mirroring
Discriminator.StringForm -- same meaning, two spellings, a flag recording
which, excluded from JSON and YAML. Components.Schemas stays
map[string]*Schema, so no public signature changes. nil is the object
form; non-nil carries the value, because `true` and `false` are opposite
schemas rather than a present/absent pair.
All three decode paths handle it, and they had to be fixed separately:
YAML in schema_yaml.go, JSON in schema_json.go, and decodeFromMap via a
new decodeSchemaValue helper wired into the decode generator. The third
one mattered -- with ResolveRefs enabled, boolean schemas were silently
dropped rather than erroring, so fixing only the first two would have
shipped a feature that vanishes on one route.
The driftguard caught three further gaps, all real:
- joiner's deep comparison called `true` and `false` equivalent, so
semantic deduplication would have merged two opposite schemas
- the structural hash ignored the field, putting them in one bucket
before comparison could reject the merge
- the marshal guard needed the exclusion entry that records why a
boolean reshapes the output to a scalar instead of dropping a key
The deepcopy generator is driven by a hand-maintained field list, so
BoolForm silently fell out of DeepCopy and would have aliased the pointer
between a schema and its copy. Added there too, with a test asserting the
clone does not share it.
Boolean schemas are gated to OAS 3.1+; 3.0 and 2.0 predate the dialect
and now report a version error rather than accepting one silently. Same
division of labour as validateDiscriminatorForm -- the parser cannot see
the document version, so the validator owns the rejection.
hashSchema crossed the cyclomatic limit with the addition, so the
Enum/Const/Required block moved into a helper, matching the per-group
helpers already in that file. Write order is unchanged, so hashes are
identical.
OAS 3.2 positive fixtures: 33/37 -> 34/37. No corpus verdict changes.
The $ref-form Callback Object, the other half of #447, is not addressed
here -- see the issue for why it needs its own approach.
Refs #447
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds boolean-form schema support. It updates schema parsing, serialization, generated decoding, deep copying, hashing, equivalence, marshal checks, and OpenAPI version validation. Tests cover JSON/YAML input, references, round trips, equality, copying, decoding, hashing, and version gates. ChangesBoolean schema support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant OpenAPI
participant GeneratedDecoder
participant decodeSchemaValue
participant Schema
participant Validator
OpenAPI->>GeneratedDecoder: Provide boolean schema value
GeneratedDecoder->>decodeSchemaValue: Decode schema-valued field
decodeSchemaValue->>Schema: Store BoolForm
Schema-->>GeneratedDecoder: Return boolean schema
Validator->>Schema: Check boolean form
Validator-->>OpenAPI: Accept or reject by version
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
erraggy
added a commit
that referenced
this pull request
Aug 1, 2026
A3's two halves had very different costs, so the plan now names them separately. A3a (bare-boolean schemas) landed additively in PR #450 and took the positive suite to 34/37. A3b ($ref-form Callback Objects) is still open. D-009 records the maintainer's constraints on the whole project: conformance fixes must be additive, no major release will be cut to accommodate them, and releases are grouped rather than issued per epic. D-010 revisits the callback half. The first analysis concluded it could only be modeled by breaking walker.CallbackHandler, which D-009 rules out, and it was headed for non-goal status. A parallel CallbackRefs field is additive and was missed the first time, so it is back in scope. The plan doc now carries all five options examined and why four were rejected, plus the frequency data -- no corpus spec uses callbacks at all, which argues both against a permanent wart and against leaving a parse failure in place. Root cause recorded for a cold reader: Callback is the only one of OAS 3.2's nine -or-reference unions whose object is not a fixed-field struct. It is an open map keyed by runtime expressions, so $ref shares a key namespace with user-authored keys, and a named Go map type has no field to put Ref in.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #450 +/- ##
==========================================
+ Coverage 86.78% 86.80% +0.01%
==========================================
Files 202 203 +1
Lines 29264 29344 +80
==========================================
+ Hits 25397 25472 +75
- Misses 2553 2557 +4
- Partials 1314 1315 +1
🚀 New features to boost your workflow:
|
![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
🤖 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 `@internal/schemautil/hash.go`:
- Around line 65-74: Update hashSchemaOrBool and the visible IsBool
boolean-schema branch to use the same boolschema:-prefixed encoding for raw bool
values and parser.Schema boolean values. Preserve distinct true and false
encodings while ensuring equivalent representations hash identically.
In `@joiner/equivalence.go`:
- Around line 396-416: Update isEmptySchema or the early empty-schema handling
in CompareSchemasWithOptions so schemas with a non-nil BoolForm are not
classified as empty, allowing the IsBool comparison block to handle them. Add
joiner tests covering true/true and false/false equivalence, true/false
differences, and boolean-versus-object differences.
In `@parser/bool_schema_test.go`:
- Around line 81-85: The IsBool test cases currently omit the nil receiver
contract. Extend the test covering Schema.IsBool to call it on a nil *Schema and
assert it returns (false, false), while preserving the existing non-nil cases.
- Around line 3-8: Update parser/bool_schema_test.go imports and
validator/bool_schema_test.go imports to include stretchr/testify/assert and
require. In both boolean-schema test files, replace setup or prerequisite checks
using t.Fatalf and t.Fatal with require assertions, and replace value or result
checks using t.Errorf and t.Error with assert assertions.
In `@validator/bool_schema_test.go`:
- Around line 25-45: Update the OAS 3.1 and 3.2 boolean component-schema
fixtures in the relevant bool schema tests to include the required paths: {}
field. In the validation assertions around the wantErr false path, also require
result.Valid to be true so positive cases verify complete document validation,
not only absence of the boolean-schema error.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 00fcd6e1-e40c-4dde-8003-72f44e18d09d
📒 Files selected for processing (15)
internal/codegen/decode/main.gointernal/codegen/deepcopy/main.gointernal/driftguard/marshal_test.gointernal/schemautil/hash.gojoiner/equivalence.goparser/bool_schema_test.goparser/decode_helpers.goparser/schema.goparser/schema_equals.goparser/schema_json.goparser/schema_yaml.goparser/zz_generated_decode.goparser/zz_generated_deepcopy.govalidator/bool_schema_test.govalidator/schema.go
Review findings on #450. Two of them are the same mistake: a second representation of an existing concept was added and only the paths I was looking at got updated. parser.Schema.Equals learned the new case; the joiner's comparison, its empty-schema gate and the hash encoding did not. hashSchemaOrBool wrote a bare "true"/"false" while hashSchema writes a "boolschema:" marker for Schema.BoolForm, so the two representations of the same boolean schema hashed differently: Items: true -> 0x358018255760441f Items: NewBoolSchema(true) -> 0x7e417e460928998a Equivalent schemas in different deduplication buckets never get compared. Both now share writeBoolSchema. isEmptySchema treated a schema carrying only BoolForm as empty, so CompareSchemasWithOptions took the early return before the IsBool comparison could run. Two identical `true` schemas came back Equivalent=false with an *empty* Differences slice -- not just a wrong verdict but an incoherent result, since nothing had been compared. A boolean schema is the opposite of empty: `false` rejects every instance and `true` accepts every instance, both deliberately. The driftguard missed this because its base schema carries Type and Properties, so the seeded value never reached the empty path. Tests were the real gap. joiner had no boolean-schema coverage at all -- the existing equality tests exercise parser.Schema.Equals, which is a different implementation from the one deduplication consults. Added coverage there, including that a mismatch reports a difference rather than an empty result, and in schemautil for the two representations agreeing. Also from review: the new tests now use testify, matching the convention in 72 of the 87 test files in these packages and the project's own gen-test skill; Schema.IsBool gains a table covering its documented nil receiver; and the validator's accepting cases assert result.Valid rather than only the absence of one message. Not taken: the suggestion to add `paths: {}` to the 3.1 and 3.2 fixtures on the grounds that `paths` is required. It is not required for 3.1+ when `components` is present -- that is the false positive #433 removed -- and those fixtures already validate clean. Adding it would have deleted the components-only coverage. Refs #447
These two files landed in #448 using plain t.Errorf and t.Fatalf. They were the only test files in parser/ and validator/ not using testify -- 72 of the 87 do, and the project's own gen-test skill calls for `require` for fatal checks and `assert` for non-fatal ones, naming cmd/oastools/commands/ as the single documented exception. Surfaced by review on #450, which flagged the same thing in the new boolean-schema tests. Converting these alongside keeps the convention whole rather than leaving two known stragglers behind. Behavior is unchanged: same assertions, same cases, 10414 tests before and after. The root-requirement loop reads a little differently -- the wantErr branch now returns early instead of falling through to the negative checks -- because the two assertions were mutually exclusive and expressing that with an early return is clearer than nesting. Refs #447
…itions
Findings from a CodeRabbit CLI review. The previous commit guarded only
the top-level entry point, so every nested position was still unguarded:
{p: true} vs {p: false} -> Equivalent=true
items: true vs items: NewBoolSchema(true) -> Equivalent=false
The first would have let semantic deduplication merge a schema whose
property accepts every instance with one whose property accepts none. It
slipped through because compareCommonFields finds nothing set on either
side -- a boolean schema has no fields -- so "no differences" read as
equivalent.
The second is the mirror of the hash defect fixed in 5f2fece. That commit
unified the hash so both representations of a boolean schema land in one
deduplication bucket; the comparison then rejected the merge anyway, so
the unification achieved nothing.
Root cause was three near-identical schema-or-bool comparators --
compareSchemaOrBool, compareItemsSchemas and comparePolymorphicSchemas.
5f2fece patched one and missed two. Rather than write the same logic a
third and fourth time, compareBoolOperands is extracted and all three now
call it, and compareDeep gains equalBoolForms for nested *Schema
positions. The top-level check collapses into the same helper, so there
is one implementation rather than five.
Coverage added for every schema-or-bool field (items, additionalProperties,
additionalItems, unevaluatedProperties, unevaluatedItems) in both
representations, for nested properties, and for the three shapes the
decode generator emits -- a map value, a singular pointer field, and a
slice element -- driven through the ResolveRefs path that reaches
decodeFromMap.
Refs #447
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
joiner/equivalence.go (1)
1298-1356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the now-unreachable "Both booleans" blocks.
In
compareItemsSchemas(Lines 1330-1345) andcomparePolymorphicSchemas(Lines 1442-1455), theleft.(bool)/right.(bool)type-assertion block is dead code.boolSchemaOperandreturnsok=truefor every rawbooloperand, so whenever bothleftandrightare raw booleans,compareBoolOperandsat line 1316 (respectively line 1430) already resolves the comparison — either matching with no difference or appending a mismatch difference — and returnstrue, which causes the caller toreturnbefore reaching the "Both booleans" block below it.
compareSchemaOrBoolalready had its equivalent duplicated branch removed in this same change. Remove the leftover blocks in these two functions for consistency, and to avoid a future edit mistakenly treating them as live code.♻️ Proposed fix
@@ compareItemsSchemas // Both schemas leftSchema, leftIsSchema := left.(*parser.Schema) rightSchema, rightIsSchema := right.(*parser.Schema) if leftIsSchema && rightIsSchema { path.push("items") compareDeep(leftSchema, rightSchema, path, result, visited, compareDocs) path.pop() return } - // Both booleans - leftBool, leftIsBool := left.(bool) - rightBool, rightIsBool := right.(bool) - if leftIsBool && rightIsBool { - if leftBool != rightBool { - path.push("items") - result.Differences = append(result.Differences, SchemaDifference{ - Path: path.String(), - LeftValue: leftBool, - RightValue: rightBool, - Description: "items boolean value mismatch", - }) - path.pop() - } - return - } - // Type mismatch@@ comparePolymorphicSchemas // Both schemas leftSchema, leftIsSchema := left.(*parser.Schema) rightSchema, rightIsSchema := right.(*parser.Schema) if leftIsSchema && rightIsSchema { compareDeep(leftSchema, rightSchema, path, result, visited, compareDocs) return } - // Both booleans - leftBool, leftIsBool := left.(bool) - rightBool, rightIsBool := right.(bool) - if leftIsBool && rightIsBool { - if leftBool != rightBool { - result.Differences = append(result.Differences, SchemaDifference{ - Path: path.String(), - LeftValue: leftBool, - RightValue: rightBool, - Description: "boolean value mismatch", - }) - } - return - } - // Type mismatchAlso applies to: 1414-1464
🤖 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 `@joiner/equivalence.go` around lines 1298 - 1356, Remove the unreachable “Both booleans” type-assertion blocks from compareItemsSchemas and comparePolymorphicSchemas. Keep compareBoolOperands as the sole handler for raw boolean operands, and leave the subsequent type-mismatch handling unchanged.
🤖 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 `@joiner/equivalence.go`:
- Around line 772-795: Update equalBoolForms so SchemaDifference.LeftValue and
RightValue store printable boolean values, dereferencing BoolForm when the
corresponding schema is boolean and using nil otherwise. Preserve the existing
mismatch detection and description behavior.
---
Outside diff comments:
In `@joiner/equivalence.go`:
- Around line 1298-1356: Remove the unreachable “Both booleans” type-assertion
blocks from compareItemsSchemas and comparePolymorphicSchemas. Keep
compareBoolOperands as the sole handler for raw boolean operands, and leave the
subsequent type-mismatch handling unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 080657ea-edec-4824-84c0-ed2c0259ece8
📒 Files selected for processing (9)
internal/schemautil/bool_schema_hash_test.gointernal/schemautil/hash.gojoiner/bool_schema_test.gojoiner/equivalence.goparser/bool_schema_test.goparser/conformance_relaxed_test.goparser/decode_schema_value_test.govalidator/bool_schema_test.govalidator/conformance_items_test.go
…ches
Two findings from a further CodeRabbit review, one of them outside the
diff range.
SchemaDifference.LeftValue and RightValue exist to be displayed, and both
new boolean sites stored something that does not survive being displayed.
equalBoolForms stored the *bool, which renders as a pointer address; the
review caught that one. compareBoolOperands stored the raw any, which for
a wrapped operand renders as the entire Schema struct -- worse, and in the
more common path:
before Left=0x46c7ff40ef38 Right=&{ <nil> [] <nil> ... map[] 0x...}
after Left=true Right=false
Both now go through boolDifferenceValue, with nil marking a side that is
not a boolean schema, matching how the other comparators leave a value
absent rather than inventing one.
The out-of-diff finding is correct: the "Both booleans" blocks in
compareItemsSchemas and comparePolymorphicSchemas became unreachable when
387b9ea put compareBoolOperands in front of them. compareBoolOperands
returns false only when neither operand is boolean, and boolSchemaOperand
reports ok for every raw bool, so code after the guard runs only when
neither side is a raw bool -- while those blocks require both to be.
Verified by that reasoning and by instrumenting both with a panic, which
10452 tests did not reach. compareSchemaOrBool lost its equivalent branch
when the helper was extracted; this finishes the job.
The new test asserts the type rather than the rendered string. An earlier
draft checked the output for "0x" and "&{", which tested how fmt formats
pointers rather than what the code stores, and was redundant besides --
assert.Equal is type-strict, so a *bool already fails it. Confirmed the
test is not vacuous by reverting the fix: it fails on every case with
"must be a plain bool, not the pointer it was read from".
Refs #447
erraggy
added a commit
that referenced
this pull request
Aug 2, 2026
PR #450 merged as 9ca0e47 after five review rounds. main is at 34/37 positive, 18/29 negative. The board was showing zero items in every column despite carrying 18. The cause is that its default view filters iteration:@current and adding an item to a project does not assign an iteration, so nothing matched. Status was fine -- GitHub's built-in workflows had already moved #433 to Done on merge. Both mechanics are now in Environment notes along with the Status and Iteration field IDs, and a warning that iteration IDs are a snapshot rather than a constant. Next actions re-ordered around what is actually Ready, and #447 flagged for a disposition: it is the A3 umbrella with no remaining work of its own now that A3a is merged and A3b is tracked in #451.
erraggy
added a commit
that referenced
this pull request
Aug 2, 2026
Each of #447's four acceptance criteria was mapped to a home before closing, and the two claimed-done ones re-verified on main rather than taken on trust. Two were delivered by #450, one goes to #451 outright, and the deep-copy/equality one splits across both. No orphaned work. Records the filing lesson too: pairing the two defects in one issue was a mistake, since they shared a discovery rather than a solution. The boolean half had an obvious additive design and the callback half did not, which is a difference worth noticing when an issue is written rather than when it is closed.
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.
Addresses the first half of #447. 🧩
JSON Schema 2020-12 allows
true(accept anything) andfalse(accept nothing) wherever a schema is expected, and OAS 3.1 adopted that dialect wholesale — soMySchema: trueis a legal Components entry. oastools rejected it outright, and rejected the whole document with it:This was masked until #433 landed: the root-requirement false positive rejected the fixture first, so the parse failure never surfaced.
OAS 3.2 positive fixtures: 33/37 → 34/37. No corpus verdict changes.
Additive, not breaking
Schema.BoolForm *bool, mirroring the existingDiscriminator.StringFormidiom — same meaning, two spellings, a flag recording which, excluded from JSON and YAML.Components.Schemasstaysmap[string]*Schema, so none of its ~88 call sites move and no public signature changes.nilis the ordinary object form. Non-nil carries the value, becausetrueandfalseare opposite schemas rather than a present/absent pair. Public helpersIsBool() (value, ok bool)andNewBoolSchema(bool).All three decode paths, fixed separately
schema_yaml.go(YAML)schema_json.go(JSON)decodeFromMap(ResolveRefs)The third one is why this isn't a two-line change. With
ResolveRefs=truethe map-driven decoder skipped any non-map value, so boolean schemas vanished with no error at all —len(Components.Schemas) == 0. Fixing only YAML and JSON would have shipped a feature that silently disappears on one route. Handled with adecodeSchemaValuehelper wired into the decode generator, so it applies to every Schema position rather than justcomponents.schemas.The driftguard earned its keep
It failed three ways on the first run, all genuine:
joiner's deep comparison calledtrueandfalseequivalent — semantic deduplication would have merged two opposite schemas.Discriminator.StringFormprecedent with the reason, not skipped.Separately: the deepcopy generator runs off a hand-maintained field list, so
BoolFormsilently fell out ofDeepCopyand*out = *inwould have aliased the pointer between a schema and its copy. Added there too, with a test asserting the clone doesn't share it. Worth noting as an argument for #439's matrix-driven generation — this is failure mode A living inside our own codegen.Version gating
Boolean schemas are 3.1+. OAS 3.0 is based on an earlier draft where a schema is always an object, and 2.0 more so, so both now report a version error rather than silently accepting one. Same division of labour as
validateDiscriminatorForm: the parser can't see the document version, so the validator owns the rejection.Incidental
hashSchemacrossed the cyclomatic limit with the addition, so the Enum/Const/Required block moved into a helper — matching the per-group helpers already in that file, rather than adding anolint. Write order is unchanged, so hashes are byte-identical to before.Testing
make checkgreen at 10,390 tests. New coverage:parser/bool_schema_test.go— decoding in both source formats; theResolveRefspath; round-tripping back to a bare scalar in YAML and JSON; equality across the six pairs that matter (true≠false, neither equals{}); deep copy not aliasing the pointer; and that a quoted"true"is a string, not a boolean schemavalidator/bool_schema_test.go— the version gate accepting at 3.1/3.2 and rejecting at 3.0/2.0, at both the Components and nested-propertiespositionsVerified: 3.2 positive suite 33/37 → 34/37, negative unchanged at 18/29, corpus error counts identical across all 10 specs.
Not in this PR
The
$ref-form Callback Object, the other half of #447. It needs a different approach and is being scoped separately — see the issue.