[STIP-28][Feature][Zeta] Enhance OptionRule to support rich declarative validation beyond presence checks

[nzw921rx]

Purpose

Closes #10976

Description

The current OptionRule + Condition system only supports presence checks and equality conditions. This forces connector developers to write imperative if/else validation scattered across *Config.java and Source/Sink constructors — invisible to REST API, CLI, Web UI, --check offline validation, and AI-assisted config generation.

This PR enhances the existing required() / optional() / conditional() methods with Condition-accepting overloads so that value constraints can be declared directly in optionRule().

What's changed

ConditionOperator enum (new) — 30 operators with self-describing metadata (category, arity, source, displaySymbol), covering:

• Numeric: >, >=, <, <=
• String: is not blank, starts with, ends with, contains, matches, is uppercase, is lowercase
• String length: length ==, length >=, length <=
• Collection: is not empty, has unique elements, size ==, size >=, size <=
• Cross-field: < [field], <= [field], > [field], >= [field], == [field], != [field], size == [field]

Condition class — 27 new static factory methods + defensive constructor validation (null operator, missing compareOption for FIELD ops, missing expectValue for BINARY+LITERAL ops) + circular chain detection in and()/or().

OptionRule.Builder — 6 new overloads:

Method	Purpose
required(Option, Condition, Condition...)	Single-field required + value constraints
required(Option, Option, Condition, Condition...)	Two-field required + cross-field constraints
optional(Option, Condition, Condition...)	Optional + value constraints (validated when present)
optional(Option, Option, Condition, Condition...)	Two-field optional + constraints
conditional(Option, T, Condition, Condition...)	Conditionally triggered value constraints
conditional(Option, T, Option, Option, Condition, Condition...)	Conditional two-field + constraints

Usage example

OptionRule.builder()
    .required(PORT,
        Condition.greaterOrEqual(PORT, 1)
            .and(Condition.lessOrEqual(PORT, 65535)))
    .required(HOST, Condition.notBlank(HOST))
    .required(START_TS, END_TS,
        Condition.lessThanField(START_TS, END_TS))
    .optional(MODE)
    .conditional(MODE, StartMode.TIMESTAMP,
        Condition.greaterThan(TIMESTAMP_VALUE, 0))
    .build();

Test Plan

./mvnw -B -pl seatunnel-api test -Dtest=ConfigValidatorTest
./mvnw -B -pl seatunnel-engine/seatunnel-engine-server test -Dtest=OptionRulesServiceTest

[nzw921rx]

@davidzollo @zhangshenghang Can you help review it? Any suggestions would be greatly appreciated. If there are any design issues, you can move on to issue: #10976 😄

[nzw921rx]

@yzeng1618 Can you help confirm if the support of OptionRulesService to return transform has a negative impact on AI generated configuration? Thank you very much.

[DanielLeens]

Thanks for working on this. I reviewed the full current head from the real validation path instead of only reading the new utility classes.

What this PR fixes

• User pain: plugin authors can describe whether an option is required, bundled, or conditional today, but they still cannot declaratively say "this value must be > 0" or "field B must be greater than field A" in OptionRule itself.
• Proposed fix: this PR adds ConditionOperator, ConditionEvaluators, OptionRule.valueConstraints, and REST metadata exposure so those validations can run during config validation.
• One-line summary: the direction is good, but the latest head still has two framework-level correctness holes before it is safe to merge.

Call chain I traced

connector config validate
  -> SeaTunnelConfValidateCommand.execute()
     -> ConfigValidator.of(readonlyConfig).validate(factory.optionRule())
        -> required / optional / conditional checks
        -> valueConstraints loop [ConfigValidator.java:174-176]
           -> ConditionEvaluators.evaluate(...)
     -> ConfigValidator.validateUnknownKeys(readonlyConfig, factory.optionRule(), pluginName)
        -> collectDeclaredKeys(rule) [ConfigValidator.java:117-129]
        -> validatePaths(...) [ConfigValidator.java:96-114]

Findings

1. Blocker: keys referenced only from valueConstraints are still rejected as unknown keys.
   At seatunnel-api/.../ConfigValidator.java:78-93,117-129, collectDeclaredKeys() only collects required / optional options and nested conditional option rules. It does not collect rule.getValueConstraints() references. So a rule like the OptionRule example itself (Condition.greaterThan(TIMESTAMP_VALUE, 0)) is accepted by validate(), but once the user actually provides timestamp_value, validateUnknownKeys() rejects it as an unknown key. That means part of the new declarative constraint feature cannot be used on the normal config-validation path.

2. Blocker: numeric comparison loses correctness for large integers.
   At seatunnel-api/.../ConditionEvaluators.java:215-223, every Number comparison is normalized through Double.compare(number.doubleValue(), ...). That is fine for ports or small counters, but it is not safe for large long values such as timestamps / offsets / IDs beyond the 2^53 precision boundary. In those cases different integers can collapse to the same double, so greaterThan / lessThan / field-to-field comparisons can silently return the wrong answer.

3. Non-blocking: the public Javadoc example calls a method that does not exist.
   seatunnel-api/.../OptionRule.java:67-74 uses Condition.range(TIMEOUT, 1000, 60000), but there is no Condition.range(...) factory in the codebase. That will mislead the next caller even though it does not affect runtime behavior.

Tests and CI

The new tests cover many happy-path constraint cases, which is a good step. What is still missing is:

• a regression test for validateUnknownKeys() + valueConstraints
• a regression test for large-integer comparison precision

The good news is the current GitHub Build is green, so the remaining blockers here are source-level correctness blockers, not CI noise.

Merge conclusion

Conclusion: can merge after fixes.

Must fix before merge

• Finding 1: include valueConstraints-referenced options (and compare-field options when present) in the declared-key set used by validateUnknownKeys().
• Finding 2: stop normalizing every Number through double for ordering comparisons.

Nice to clean up

• Finding 3: update the OptionRule example to a real API shape.

Happy to re-review once you push the next revision.

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from scratch against the full current validation path.

What this PR solves

• User pain: connector authors can declare required/optional/exclusive relationships today, but cannot describe richer value constraints directly in OptionRule.
• Fix approach: add declarative ConditionOperator / ConditionEvaluators support, wire it into ConfigValidator, and expose the metadata through the REST response.
• One-line summary: the two framework-level blockers from my previous round are fixed on the latest head, and I did not find a new blocking issue on the current code.

Runtime path I checked

connector config validate
  -> ConfigValidator.validate(rule)
      -> validate required/optional/single-choice rules
      -> validate valueConstraints
          -> ConditionEvaluators.evaluate(...)
  -> ConfigValidator.validateUnknownKeys(...)
      -> collectDeclaredKeys(rule)
      -> includes valueConstraints + compareOption keys

Re-review result

• The old unknown-key blocker is fixed: collectDeclaredKeys() now includes value-constraint keys and compare-option keys, so constraint-only options are no longer rejected as unknown.
• The old large-integer blocker is fixed for the real timestamp/offset-style path I called out before: compareNumbers() now avoids routing Long comparisons through double.
• The Javadoc example was also corrected to use real APIs.

Tests / CI

• The new regression coverage around ConfigValidatorTest is much better now.
• The current GitHub Build is green.
• I did not see a flaky-test pattern in the added UTs.

Conclusion: can merge

1. Blocking items

• None from my side on the latest head.

2. Suggested non-blocking follow-up

• If this condition DSL grows further later, it may be worth adding one focused regression around decimal-style numeric comparison semantics as a future hardening step, but that is not a blocker for this PR.

[yzeng1618]

@yzeng1618 Can you help confirm if the support of OptionRulesService to return transform has a negative impact on AI generated configuration? Thank you very much.

I took a rough look at the CLI (#10789) consumption side and wanted to double-check two points with you — I may not have the full picture, so please help confirm.
First, on transform: in connectors.py, get_connector_detail() currently defaults query_types to ["source", "sink"] only, so transform doesn't seem to be requested at this stage. So even if OptionRulesService now returns transform, the CLI side likely won't use it yet — supporting it properly would probably need some follow-up adaptation on the CLI side.
Second, on the new valueConstraints field in this PR: from what I can tell it's just returned in the response for now, and the CLI side doesn't yet have parsing/validation logic to consume it. So it shouldn't affect the existing AI config generation, but I'd appreciate it if you could confirm whether I'm missing anything @SEZ9

[DanielLeens]

Thanks for checking the downstream CLI side as well.

Your read is consistent with what I see on the current PR head d6b235ac9364848695b5d1d2587e9ab13c342d37: OptionRulesService returning transform metadata is additive on the service side, while the current CLI path still requests only source / sink, and the new valueConstraints payload is not being consumed there yet.

Since there is still no new commit after the latest Daniel review on this head, I am keeping this as a reply-only follow-up rather than reopening a full review. From Daniel's side I am not adding a new blocker based on this CLI discussion on the current revision.

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from scratch again against the full current validation path.

What this PR solves

• User pain: OptionRule can describe presence rules today, but not richer constraints like > 0 or field A < field B in a declarative way.
• Fix approach: add ConditionOperator / ConditionEvaluators, wire valueConstraints into ConfigValidator, and expose the constraint metadata through REST.
• One-line summary: the two framework-level blockers from my earlier rounds are fixed on the latest head, and I did not find a new blocking issue.

Runtime path I checked

connector config validate
  -> ConfigValidator.validate(rule)
      -> required / optional / conditional checks
      -> valueConstraints checks
          -> ConditionEvaluators.evaluate(...)

unknown key check
  -> ConfigValidator.validateUnknownKeys(...)
      -> collectDeclaredKeys(rule)
      -> now includes valueConstraints + compareOption keys

REST metadata exposure
  -> OptionRulesService
  -> OptionRuleResponse

Re-review result

• The old unknown-key blocker is fixed: keys referenced only from valueConstraints are now included in the declared-key set.
• The old large-integer precision blocker is fixed: Long comparisons no longer route through double.
• The new docs/tests line up with the current runtime path from what I checked.

Tests / stability

• The updated unit coverage is much stronger now.
• I did not see a flaky-test pattern in the added or changed tests.
• The current GitHub Build is still running, but I do not have a source-level blocker to add on top of the latest code.

Conclusion: can merge

1. Blocking items

• None from my side on the latest head.

2. Suggested non-blocking follow-up

• If this DSL grows further later, it may be worth documenting the intended behavior for mixed numeric types more explicitly, but that is not a blocker for this PR.

From the code-review side this looks good now.

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from scratch again, this time focusing on the actual delta after my June 1 review instead of assuming the earlier LGTM still held.

What problem this PR solves

• User pain: OptionRule can express presence rules today, but not richer value-level constraints like > 0, not blank, or start < end as part of the contract itself.
• Fix approach: add declarative Condition / ConditionOperator / Conditions support, run those constraints through ConfigValidator, and expose the metadata through the REST option-rule path.
• One-line summary: this turns value validation into a first-class part of the SeaTunnel config contract instead of leaving it scattered in ad hoc runtime checks.

Full runtime path I checked

plugin defines OptionRule
  -> required / optional / conditional declarations
  -> valueConstraints attached to the rule

user config validation
  -> ConfigValidator.validate(rule)
     -> required / optional / conditional checks
     -> valueConstraints checks
        -> ConditionEvaluators.evaluate(...)
           -> numeric / string / collection / field-to-field evaluation

unknown-key guard
  -> ConfigValidator.validateUnknownKeys(...)
     -> collectDeclaredKeys(rule)
     -> collectConditionKeys(condition)

REST metadata exposure
  -> OptionRulesService
  -> OptionRuleResponse

Core logic re-review

I rechecked the current implementations in:

• seatunnel-api/.../Condition.java
• seatunnel-api/.../ConditionEvaluators.java
• seatunnel-api/.../ConditionOperator.java
• seatunnel-api/.../ConfigValidator.java
• seatunnel-api/.../OptionRule.java
• seatunnel-engine/.../OptionRulesService.java
• seatunnel-engine/.../OptionRuleResponse.java

The two framework-level blockers I raised in earlier rounds still remain fixed on the current head:

• keys referenced only from valueConstraints are still collected into the declared-key set, so they are no longer rejected by validateUnknownKeys();
• large integer comparisons still avoid the old “force everything through double” precision problem.

The latest delta is mostly API cleanup (Condition + Conditions usage), broader ConfigValidatorTest coverage, and doc refresh. After tracing the real validation path again, I did not find a new source-level correctness blocker in the current code.

Compatibility / side effects

• Compatibility: fully compatible from my review perspective. This is additive behavior, not a breaking change to existing option definitions.
• Performance / side effects: the extra work stays on the config-validation path, so I do not see a CPU / memory / concurrency blocker.
• Error handling: the wrapped validation messages in ConditionEvaluators.evaluate() are now easier to diagnose.

Tests / stability

• The expanded UT coverage is materially better now, especially around declared-key collection, cross-field comparisons, and applicability rules.
• I did not see a flaky-test pattern in the new unit tests. No Thread.sleep, weak async assertions, or environment-sensitive resource usage showed up in the added test code.
• Stability rating: Stable.

Existing review context

I also checked the current community context again. I do not have another maintainer/reviewer blocker to add on top of the latest code, and my current conclusion is still aligned with the earlier “source path looks good” direction.

Current blocker outside the source path

The remaining gate is CI, not a new code-path issue from my side:

• GitHub still shows Build as failed on the current head.
• From the current check metadata I could confirm at least Run / unit-test (11, ubuntu-latest) failed, and several sibling lanes were cancelled afterward.
• When I checked, the contributor-side workflow was still not exposing the final failed-step log yet, so I cannot honestly pin that failure to a specific new source regression from this revision.

Merge conclusion

Conclusion: can merge after fixes.

1. Blocking items

• Please get the current Build back to green first, starting with the failing Run / unit-test (11, ubuntu-latest) lane.

2. Suggested follow-up

• Once that failing lane exposes its final stack trace, we can tell whether this is just CI noise or a real regression from the latest revision. From the code-review side, I do not have a new blocker beyond the red build gate.

Overall, the current head still looks coherent on the real validation path, and the previous Daniel correctness blockers remain closed.

[davidzollo]

Re-review (after commits 728e3ee...894091d)

Apologies for being less thorough on the first pass — re-evaluating the whole PR from scratch as required, and acknowledging that I had under-credited the depth of the rewrite. Local verification this time: 146/146 tests pass in seatunnel-api (ConfigValidatorTest 143 + ConditionTest 3), spotless:check clean.

What this PR really does (concrete example)

Today, "port must be in [1, 65535]" or "start_ts must be less than end_ts" lives as imperative if/else inside *Config.java, invisible to REST/CLI/Web UI/AI tooling, and reported one-at-a-time. After this PR:

import static org.apache.seatunnel.api.configuration.util.Conditions.*;

OptionRule.builder()
    .required(PORT, greaterOrEqual(PORT, 1).and(lessOrEqual(PORT, 65535)))
    .required(HOST, notBlank(HOST))
    .required(START_TS, END_TS, lessThanField(START_TS, END_TS))
    .build();

ConfigValidator runs every constraint at job-submission time, aggregates all failures (structural + value) into a single Option validation failed (N errors): [1] ... [2] ... message, REST /option-rules exposes both the symbolic compareOperator and the structured conditionOperator / conditionOperatorCategory so frontends/AI don't need to string-match.

Verifying the previous round's blockers were fixed

All 8 items I raised in the previous review are addressed:

#	Previous finding	Status	How
1	optional(start, end, lessThanField(start, end)) threw Cannot compare null when one side missing	Fixed	isConstraintApplicable now splits the chain at OR boundaries and requires AND-segments to be fully present; tests testOptionalCrossFieldOnlyStartPresent / OnlyEndPresent / NoFalsePositive pin it down
2	greaterOrEqual(PORT,1).or(notBlank(HOST)) broke short-circuit when PORT null	Fixed	All numeric/FIELD evaluators are now null-safe (v != null && ...); covered by testOrChainNumericNullWithStringFallback (4 scenarios)
3	greaterThan(longOpt, 0) raised Numeric type mismatch because int vs Long	Fixed	compareNumberValues now normalizes via BigDecimal / double / long three-tier; testNumericTypeMismatchIntVsLong / DoubleVsInt cover it
4	REST ConditionNode only had symbol string; AI/frontend forced to string-match	Fixed	Added conditionOperator (e.g. GREATER_OR_EQUAL) + conditionOperatorCategory (e.g. NUMERIC)
5	Raw-type new Condition(...) factories	Fixed	Factories moved to Conditions.java (clean generics); narrow Condition.java to data + chain logic
6	Evaluator exceptions dropped option key	Fixed	ConditionEvaluators.evaluate wraps inner exceptions as Failed to evaluate constraint '%s' on option '%s': %s
7	Condition.of(opt, null) API narrowed without incompatible-changes.md entry	Fixed	Entry added in both docs/{en,zh}/introduction/concepts/incompatible-changes.md
8	Test coverage gaps (optional cross-field single-side, OR with numeric, type mismatch)	Fixed	Test count went from 32 → 143 in ConfigValidatorTest; nearly every scenario I called out has an explicit test

Bonus improvement that wasn't in the previous review but is genuinely production-grade: collectErrors aggregates structural + value errors into one OptionValidationException instead of fail-fast. Big UX win for users debugging large HOCON configs.

New findings on this pass

Issue 1 (medium) — Asymmetric cross-field: optional(head) + required(compareField) makes the optional head implicitly required

isConstraintApplicable collects head + compareOption + all chain nodes and returns true as soon as any of them is AbsolutelyRequiredOptions. Consider:

.required(START_TS)
.optional(MAX, lessThanField(MAX, START_TS))

• allOptions = {MAX, START_TS}
• START_TS is absolutely required → first-stage hit → applicable
• FIELD_LESS_THAN(null, START_TS) → returns false
• Error reported: constraint: 'max' < 'start_ts'

User's clear intent: "MAX is optional; if set, must be less than START_TS." Actual behavior: MAX is silently required. The symmetric case (required(head) + optional(compareField)) is already pinned down by testRequiredPrimaryWithAbsentOptionalCompareField as "must fail", which is a consistent but easy-to-trip design choice.

Suggested fix (Option A, recommended): keep "required head forces applicable" but only inspect the head node, not the entire option set. That makes both directions symmetric — the head decides. Adjust testRequiredPrimaryWithAbsentOptionalCompareField accordingly (END_TS is optional, so the constraint should skip and let structural validation handle the absence).

Option B (conservative): keep both semantics, but add a Javadoc warning on every cross-field factory in Conditions.java stating "if used together with optional(head, ...), the compareOption must also be optional".

Issue 2 (low) — MetadataExportCommand didn't pick up conditionOperator / conditionOperatorCategory

OptionRulesService.toConditionNode now exposes the structured operator metadata for the REST API, but MetadataExportCommand.exportCondition still only emits compareOperator (symbol string). The PR description specifically calls out AI-assisted config generation, and the CLI export is exactly the surface AI tools consume offline. Quick fix: mirror the same fields via op.name() / op.getCategory().name().

Issue 3 (low) — Docs hint at length/size constraints that the PR didn't ship

The previous revision had LENGTH_*, COLLECTION_SIZE_*, FIELD_EQUAL/NOT_EQUAL, FIELD_SIZE_EQUAL, etc. Those have been (rightly) trimmed — the surface is now 17 operators in 4 categories. But the architecture doc still uses "string length limit" / "fixed collection size" as motivating examples. Suggest either narrowing the docs or adding a "currently supported operators" callout in Conditions.java's class-level Javadoc so users know what's actually available without grepping.

Issue 4 (medium) — OptionValidationException message format changed; no incompatible-changes.md entry

Old format (per error):

ErrorCode:[API-02], ErrorDescription:[Option item validate failed] - There are unconfigured options, the options('host') are required.

New format (aggregated):

ErrorCode:[API-02], ErrorDescription:[Option item validate failed] - Option validation failed (2 errors):
  [1] option: 'host'
      type: required
      constraint: required option is not configured
  [2] option: 'port'
      type: value
      constraint: 'port' >= 1

This is strictly a better UX, but any consumer doing getMessage().contains("are required") will break. Connector unit tests doing string assertions on the message (the PR itself rewrote a handful of them) will fail. Worth adding a second incompatible-changes.md entry next to the Condition.of(opt, null) one.

Issue 5 (low) — Condition.* static factories were removed without a deprecated bridge

Condition.greaterThan(...), Condition.notBlank(...), etc. moved to Conditions.*. Anyone using them outside the main repo will get a compile error. Either:

• Keep @Deprecated public static <T> Condition<T> greaterThan(...) { return Conditions.greaterThan(...); } bridges, or
• Document the rename in incompatible-changes.md alongside the others.

Issue 6 (low) — compareNumbers error message includes raw values

throw new OptionValidationException(
    "Cannot compare null values in numeric comparison: left=%s, right=%s", a, b);

For numeric options this is fine, but if a connector ever wires up a sensitive option as Number, the raw value leaks into logs. Safer to swap to leftType=%s, rightType=%s.

Issue 7 (low) — Conditions.java class Javadoc has a truncated example

 * <pre>{@code
 * static *
 * OptionRule.builder()

Looks like an incomplete import static org.apache.seatunnel.api.configuration.util.Conditions.*; line.

Issue 8 (low) — extractInnerMessage couples to OptionValidationException text format

Splitting on " - " to strip the ErrorCode:[API-02] - prefix is fine today, but ties the evaluator wrapper to an implementation detail of the exception. A getRawMessage() accessor on OptionValidationException would be cleaner. Not urgent.

Issue 9 (low) — Missing test for the Issue 1 scenario

testRequiredPrimaryWithAbsentOptionalCompareField pins down required(head) + optional(compareField). The mirror case (optional(head) + required(compareField), head absent) is not explicitly tested. Worth adding one to make the chosen semantic regression-proof.

Issue summary

#	Issue	Location	Severity
1	optional(head) + required(compareField) implicitly forces head to be required	ConfigValidator#isConstraintApplicable	medium
4	OptionValidationException message format change not documented	ConfigValidator#validate(OptionRule)	medium
2	MetadataExportCommand doesn't expose conditionOperator / conditionOperatorCategory	MetadataExportCommand#exportCondition	low
3	Docs imply length/size constraints that aren't shipped	Conditions.java, configuration-and-option-system docs	low
5	Condition.* static factories removed without @Deprecated bridges or doc	Condition.java	low
6	compareNumbers error message leaks raw values	ConditionEvaluators#compareNumbers	low
7	Conditions.java class Javadoc has a truncated example	Conditions.java	low
8	extractInnerMessage couples to exception text format	ConditionEvaluators#extractInnerMessage	low
9	Missing test for optional(head) + required(compareField) + head absent	ConfigValidatorTest	low

Verification commands

Command	Result	Note
./mvnw -B -pl seatunnel-api spotless:check -nsu -Dmaven.gitcommitid.skip=true	PASS	clean
./mvnw -B -pl seatunnel-api test -Dtest=ConfigValidatorTest,ConditionTest -nsu -Dmaven.gitcommitid.skip=true	PASS (146/146)
./mvnw -B -pl seatunnel-engine/seatunnel-engine-server test -Dtest=OptionRulesServiceTest -am ...	not run	seatunnel-config-shade failed to compile (missing ConfigValue/Entry symbols, unrelated to this PR — typical when shade-relocate hasn't run)
git grep "Condition\.of\([^,]*, *null" in main repo	none	no current production callers, backward-compat risk for Condition.of(opt, null) narrowing is effectively zero

Conclusion: ready to merge after two doc additions; the rest can be follow-ups

No runtime blockers remain. The remaining 9 items are either documentation gaps (4, 5), low-severity polish (2, 3, 6, 7, 8, 9), or one semantic edge case that I think deserves discussion before code change (1).

Recommended pre-merge (≈30 min):

• Add a single incompatible-changes.md block covering (a) OptionValidationException message format change and (b) Condition.* static factory relocation. Or alternatively keep @Deprecated bridges on Condition.*.

Recommended follow-up PR:

• Decide on Issue 1's semantic (Option A: head-only isConstraintApplicable, or Option B: Javadoc warning on cross-field factories).
• Mirror the structured operator metadata in MetadataExportCommand (Issue 2).
• Tighten docs scope to currently-shipped operators (Issue 3).
• Polish 6/7/8 + add the missing test from Issue 9.

Overall assessment

This is a meaningful step up from the previous revision. The contributor turned every one of the 8 prior findings into a precise fix backed by tests, plus added error aggregation and structured REST metadata that weren't requested but are genuinely production-grade. The 17-operator scope is a deliberate, healthy narrowing relative to the prior 30 — better to ship a tight set with full evaluator + REST + AI parity than 30 operators in inconsistent states.

Solid work. Looking forward to seeing this land.

[nzw921rx]

@davidzollo Thank you for your review.

I have made the following fixes based on the results, and welcome your review at any time

• isConstraintApplicable now only checks the head option for required-ness — compareField in cross-field operators no longer forces applicability. This makes optional(head) + required(compareField) behave symmetrically: head absent → constraint skipped.

• MetadataExportCommand.exportCondition now emits conditionOperator and conditionOperatorCategory, aligned with the REST API.

• Fixed truncated Javadoc in Conditions.java; added supported operator inventory (17 operators, 4 categories).

• incompatible-changes.md entry for OptionValidationException structured aggregation format.

• Skipped — Condition.* factories are new code (never released), no compat entry needed.

• compareNumbers no longer leaks raw values in error messages.

• OptionValidationException gained getRawMessage(); extractInnerMessage replaced with direct accessor.

• Added 3 new tests covering optional(head) + required(compareField) scenarios (head absent / both present / violation).

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from scratch again against the full current validation path.

What this PR solves

• User pain: OptionRule can describe presence rules today, but not richer constraints like > 0 or field A < field B in a declarative way.
• Fix approach: add ConditionOperator / ConditionEvaluators, wire valueConstraints into ConfigValidator, and expose the constraint metadata through REST.
• One-line summary: the two framework-level blockers from my earlier rounds are fixed on the latest head, and I did not find a new blocking issue.

Runtime path I checked

connector config validate
  -> ConfigValidator.validate(rule)
      -> required / optional / conditional checks
      -> valueConstraints checks
          -> ConditionEvaluators.evaluate(...)

unknown key check
  -> ConfigValidator.validateUnknownKeys(...)
      -> collectDeclaredKeys(rule)
      -> now includes valueConstraints + compareOption keys

REST metadata exposure
  -> OptionRulesService
  -> OptionRuleResponse

Re-review result

• The old unknown-key blocker is fixed: keys referenced only from valueConstraints are now included in the declared-key set.
• The old large-integer precision blocker is fixed: Long comparisons no longer route through double.
• The new docs/tests line up with the current runtime path from what I checked.

Tests / stability

• The updated unit coverage is much stronger now.
• I did not see a flaky-test pattern in the added or changed tests.
• The current GitHub Build is still queued, but I do not have a source-level blocker to add on top of the latest code.

Conclusion: can merge

1. Blocking items

• None from my side on the latest head.

2. Suggested non-blocking follow-up

• If this DSL grows further later, it may be worth documenting the intended behavior for mixed numeric types more explicitly, but that is not a blocker for this PR.

From the code-review side this looks good now.

[DanielLeens]

Thanks for the update. I re-reviewed the latest head from scratch again against the full current validation path, including the new test-only follow-up commit.

What this PR solves

• User pain: OptionRule can describe presence rules today, but not richer declarative value constraints like > 0, not blank, or field A < field B.
• Fix approach: add ConditionOperator / ConditionEvaluators, wire valueConstraints into ConfigValidator, and expose the metadata through the REST option-rule path.
• One-line summary: the framework-level correctness blockers from my earlier rounds remain fixed on the current head, and the latest commit only tightens the validation test assertion.

Runtime chain I checked

plugin defines OptionRule
  -> required / optional / conditional declarations
  -> valueConstraints attached to the rule

user config validation
  -> ConfigValidator.validate(rule)
     -> required / optional / conditional checks
     -> valueConstraints checks
        -> ConditionEvaluators.evaluate(...)

REST metadata exposure
  -> OptionRulesService
  -> OptionRuleResponse

Re-review result

• ConfigValidator.java, ConditionEvaluators.java, OptionRule.java, and the REST metadata path still line up with the intended design.
• The previous Daniel blockers around declared-key collection and large-integer comparison remain fixed on the current head.
• The latest commit only changes SeaTunnelConfValidateCommandTest.java:101-104 to assert the current unified validation message shape, which is consistent with the runtime behavior.

Tests / stability

• The updated UT coverage remains strong.
• I do not see a flaky-test pattern in the changed tests.
• The current PR checks are green from the metadata I reviewed.

Conclusion: can merge

1. Blocking items

• None from my side on the latest head.

2. Suggested non-blocking follow-up

• None.

From the code-review side this looks good to merge.

[davidzollo]

+1
LGTM