From 0a0093af3e5f0fbfe2835fb84b6681f637b9206d Mon Sep 17 00:00:00 2001 From: kary zheng Date: Mon, 31 Aug 2026 01:48:13 -0700 Subject: [PATCH] feat(visualization): require the Filled Area Plot line group under its switch Turning on Split Plot by Line Group makes the line group required, because code generation asserts it and the run ends on `Line Group cannot be empty`. The field is declared optional and carried no conditional constraint, so the property panel accepted the configuration and the error waited for the run. A field that is optional is exactly the one a user leaves behind. The schema states the rule instead, in the conditional form the Sklearn text columns already use: required under `facetColumn`, so the panel refuses the configuration while it is being written. Conditional rather than a plain required, so a freshly dropped operator, whose switch is off, is not flagged for a field it has no use for. The assertion stays as the last line of defence, and the tests that pin it stay with it. Co-Authored-By: Claude Opus 5 (1M context) --- .../filledAreaPlot/FilledAreaPlotOpDesc.scala | 23 +++++++++++++++- .../FilledAreaPlotOpDescSpec.scala | 26 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDesc.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDesc.scala index 84ca6082d24..573d5ce5009 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDesc.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDesc.scala @@ -20,7 +20,7 @@ package org.apache.texera.amber.operator.visualization.filledAreaPlot import com.fasterxml.jackson.annotation.{JsonProperty, JsonPropertyDescription} -import com.kjetland.jackson.jsonSchema.annotations.JsonSchemaTitle +import com.kjetland.jackson.jsonSchema.annotations.{JsonSchemaInject, JsonSchemaTitle} import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableString @@ -32,6 +32,27 @@ import org.apache.texera.amber.pybuilder.PythonTemplateBuilder import javax.validation.constraints.NotNull +// `lineGroup` names the column the plot is split on, so it is required exactly when +// that switch is on: with the switch off nothing reads it, and with the switch on +// code generation asserts it and the run ends. Conditional rather than a plain +// required, so a freshly dropped operator is not flagged for a field it has no use +// for. +@JsonSchemaInject(json = """ +{ + "allOf": [ + { + "if": { + "properties": { + "facetColumn": { "const": true } + } + }, + "then": { + "required": ["lineGroup"] + } + } + ] +} +""") class FilledAreaPlotOpDesc extends PythonOperatorDescriptor { @JsonProperty(required = true) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDescSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDescSpec.scala index 8f93c3fd0f0..f92d56d2bc5 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDescSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/visualization/filledAreaPlot/FilledAreaPlotOpDescSpec.scala @@ -22,6 +22,7 @@ package org.apache.texera.amber.operator.visualization.filledAreaPlot import com.typesafe.config.ConfigFactory import org.apache.texera.amber.core.tuple.{AttributeType, Schema} import org.apache.texera.amber.operator.LogicalOp +import org.apache.texera.amber.operator.metadata.OperatorMetadataGenerator import org.apache.texera.amber.util.JSONUtils.objectMapper import org.scalatest.BeforeAndAfter import org.scalatest.flatspec.AnyFlatSpec @@ -30,6 +31,7 @@ import org.scalatest.matchers.should.Matchers import java.nio.charset.StandardCharsets import java.nio.file.Files import java.util.concurrent.TimeUnit +import scala.jdk.CollectionConverters._ import scala.util.Try class FilledAreaPlotOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matchers { @@ -370,4 +372,28 @@ class FilledAreaPlotOpDescSpec extends AnyFlatSpec with BeforeAndAfter with Matc fp.facetColumn shouldBe true fp.pattern shouldBe "p" } + + // The assertion tested above is the last line of defence, reached only once the + // user has hit run. The schema is what refuses the configuration while it is + // still being written, and only under the switch: with it off nothing reads the + // field, so a freshly dropped operator is not flagged for it. + "FilledAreaPlotOpDesc JSON schema" should + "require the line group only when the plot is split by it" in { + val schema = + OperatorMetadataGenerator.generateOperatorJsonSchema(classOf[FilledAreaPlotOpDesc]) + + val baseRequired = schema.get("required").elements().asScala.map(_.asText()).toSet + baseRequired should contain("x") + baseRequired should not contain "lineGroup" + + val rule = schema + .get("allOf") + .elements() + .asScala + .find(node => node.has("if") && node.has("then")) + .getOrElse(fail("expected a conditional if/then rule in the FilledAreaPlot schema")) + rule.get("if").get("properties").get("facetColumn").get("const").asBoolean() shouldBe true + val thenRequired = rule.get("then").get("required").elements().asScala.map(_.asText()).toList + thenRequired should contain("lineGroup") + } }