Skip to content

Conversation

@jviotti
Copy link
Member

@jviotti jviotti commented Jan 28, 2026

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

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti jviotti merged commit c321100 into main Jan 28, 2026
13 checks passed
@jviotti jviotti deleted the core-mutates branch January 28, 2026 16:26
@augmentcode
Copy link

augmentcode bot commented Jan 28, 2026

🤖 Augment PR Summary

Summary: This PR upgrades the vendored sourcemeta/core dependency and updates JSON Schema transformation rules to match the new Core API expectations.

Changes:

  • Bumped core in DEPENDENCIES to 421f999…
  • Added using mutates = std::true_type/false_type; to schema transform rule classes (jsonbinpack mappers and core alterschema rules)
  • Extended SchemaTransformer to track whether each rule mutates (vector<pair<rule,bool>>)
  • Refactored shared framing + rule-checking into helpers (analyse_frame, check_rules)
  • Updated SchemaTransformer::apply to run mutating rules in the main loop and run non-mutating rules as a final check
  • Adjusted rule removal logic to account for the new rule storage structure

Technical Notes: SchemaTransformer::add now requires a T::mutates type alias and uses it to decide whether a rule participates in mutation vs. final lint-style checks.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

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

callback(entry_pointer, rule->name(), rule->message(), outcome);
continue;
}
rule->transform(current, outcome);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SchemaTransformer::apply no longer catches SchemaAbortError, but SchemaTransformRule::transform still throws by default; this means a misclassified/partial rule (or a rule that intentionally aborts) will now throw out of apply instead of reporting via callback/result.first. Is this exception-propagation change intended for callers of apply?

Fix This in Augment

🤖 Was this useful? React with ��� or 👎


/// The rule transformation. If this virtual method is not overriden,
/// then the rule condition is considered to not be fixable.
/// then the rule is considered to not mutate the schema.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says that if transform isn’t overridden the rule is “considered to not mutate the schema”, but the default SchemaTransformRule::transform implementation still throws SchemaAbortError. This seems like a contract/docs mismatch that could confuse implementers of new rules.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants