Delete type when all logical applicators have it in the canonicaliser

[jviotti]

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

[augmentcode]

🤖 Augment PR Summary

Summary: This PR extends the canonicalizer to remove redundant sibling type constraints when they’re already implied by logical applicators.

Changes:

• Introduced a new canonicalizer rule TypeSubsumedByApplicator that deletes type when every branch of a sibling anyOf/oneOf asserts the same single type.
• Registered the new rule early in the canonicalizer transform pipeline (in src/alterschema/alterschema.cc).
• Added new test coverage for 2019-09 and 2020-12 to validate that redundant top-level type is removed when subsumed by anyOf/oneOf.
• Adjusted existing Draft 4/6/7 expectations where schemas with a type sibling are now simplified directly to anyOf (since each branch already carries the same type).

Technical Notes: The rule is gated by dialect/vocabulary presence to ensure both validation (for type) and applicator (for anyOf/oneOf) support before applying the simplification.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. 1 suggestion posted.

Comment augment review to trigger a new review at any time.

[augmentcode]

The comment says the subsuming union is the “sole instance-affecting sibling”, but the code skips both anyOf and oneOf unconditionally (so a schema containing both applicators still qualifies even if only one of them actually subsumes the type). Consider tightening or rewording this comment to match the condition being enforced.

Severity: low

_{🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.}

[cubic-dev-ai]

2 issues found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/alterschema/canonicalizer/type_subsumed_by_applicator.h">

<violation number="1" location="src/alterschema/canonicalizer/type_subsumed_by_applicator.h:74">
P3: The code comment says "the subsuming union is the sole instance-affecting sibling" but the loop unconditionally skips both `anyOf` and `oneOf` regardless of which one actually subsumes `type`. If a schema contains both applicators and only one subsumes, the non-subsuming one is still skipped. The comment should be reworded to accurately describe the condition — e.g., "the only other instance-affecting siblings are the logical applicators (`anyOf`/`oneOf`)".</violation>

<violation number="2" location="src/alterschema/canonicalizer/type_subsumed_by_applicator.h:83">
P2: This guard prevents the new canonicalization whenever another assertion like `properties` is present, even though the applicator has already subsumed the sibling `type`. Remove the sibling instance-affecting check so eligible schemas are actually canonicalized.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This guard prevents the new canonicalization whenever another assertion like properties is present, even though the applicator has already subsumed the sibling type. Remove the sibling instance-affecting check so eligible schemas are actually canonicalized.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/alterschema/canonicalizer/type_subsumed_by_applicator.h, line 83:

<comment>This guard prevents the new canonicalization whenever another assertion like `properties` is present, even though the applicator has already subsumed the sibling `type`. Remove the sibling instance-affecting check so eligible schemas are actually canonicalized.</comment>

<file context>
@@ -0,0 +1,94 @@
+        continue;
+      }
+
+      if (walker(entry.first, vocabularies).instances.any()) {
+        return false;
+      }
</file context>

[cubic-dev-ai]

P3: The code comment says "the subsuming union is the sole instance-affecting sibling" but the loop unconditionally skips both anyOf and oneOf regardless of which one actually subsumes type. If a schema contains both applicators and only one subsumes, the non-subsuming one is still skipped. The comment should be reworded to accurately describe the condition — e.g., "the only other instance-affecting siblings are the logical applicators (anyOf/oneOf)".

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/alterschema/canonicalizer/type_subsumed_by_applicator.h, line 74:

<comment>The code comment says "the subsuming union is the sole instance-affecting sibling" but the loop unconditionally skips both `anyOf` and `oneOf` regardless of which one actually subsumes `type`. If a schema contains both applicators and only one subsumes, the non-subsuming one is still skipped. The comment should be reworded to accurately describe the condition — e.g., "the only other instance-affecting siblings are the logical applicators (`anyOf`/`oneOf`)".</comment>

<file context>
@@ -0,0 +1,94 @@
+
+    ONLY_CONTINUE_IF(subsumed);
+
+    // Only drop the type when the subsuming union is the sole instance-affecting
+    // sibling. Otherwise removing it would orphan another assertion (such as
+    // `properties`) that relies on the type being present
</file context>