AP-23732: Change default for Sorter/Top-K for non-String and non-RowID columns by enplotz · Pull Request #166 · knime/knime-base

enplotz · 2026-03-27T14:31:01Z

AP-23732 (Sorter node: change default of "comparison" mode for all string-compatible types (except String and RowID) and improve labels/description)

Copilot

Pull request overview

Updates Sorter/Top-K (and shared sorting utilities) to use a more suitable default “Compare as” mode for string-compatible non-String columns, and refreshes the UI wording to better explain the two comparison strategies.

Changes:

Adjust default StringComparison based on the selected column type (String/RowID → natural; otherwise → typed comparator).
Rename and re-describe the comparison modes in the dialog and node descriptions (“Compare as”).
Update settings schema snapshot expectations to reflect the new labels/descriptions/defaults.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java	Implements type-dependent default for `StringComparison` and updates labels/widget text.
org.knime.base/src/org/knime/base/node/preproc/topk/TopKSelectorNodeFactory.java	Expands node description to document the new “Compare as” behavior.
org.knime.base/src/org/knime/base/node/preproc/sorter/SorterNodeFactory.java	Expands node description to document the new “Compare as” behavior.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.topk.TopKSelectorNodeSettingsTest1.snap	Updates schema snapshot titles/descriptions for comparison mode.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.topk.TopKSelectorNodeSettingsTest.snap	Updates schema snapshot titles/descriptions for comparison mode.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.sorter.SorterNodeSettingsTest3.snap	Updates schema snapshot titles/descriptions/default for comparison mode.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.sorter.SorterNodeSettingsTest2.snap	Updates schema snapshot titles/descriptions/default + default value instances.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.sorter.SorterNodeSettingsTest1.snap	Updates schema snapshot titles/descriptions/default + default value instances.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.sorter.SorterNodeSettingsTest.snap	Updates schema snapshot titles/descriptions/default + default value instances.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.rank.RankNodeSettingsTest1.snap	Updates schema snapshot titles/descriptions for comparison mode.
org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.rank.RankNodeSettingsTest.snap	Updates schema snapshot titles/descriptions for comparison mode.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.sorter.SorterNodeSettingsTest3.snap:143

StringComparisonSetting introduces new predicates (IsActualStringOrRowId / IsOtherStringCompatible) and removes the old isStringColumn flag from SortingCriterionSettings. These snapshots still contain an isStringColumn property and UI rules referencing it, which will likely no longer match the generated schema—please regenerate/update snapshots (and any rules) to reflect the new predicate fields.

            "rule" : {
              "effect" : "SHOW",
              "condition" : {
                "scope" : "#/properties/isStringColumn",
                "schema" : {
                  "const" : true
                }
              }

org.knime.base.tests/files/test_snapshots/org.knime.base.node.preproc.topk.TopKSelectorNodeSettingsTest.snap:181

These snapshots still use an isStringColumn state field to control visibility of the comparison control. Since SortingCriterionSettings no longer defines isStringColumn (it now uses isActualStringOrRowId / isOtherStringCompatible), the snapshot schema/rules likely need to be regenerated to match the new dialog state fields.

            "rule" : {
              "effect" : "SHOW",
              "condition" : {
                "scope" : "#/properties/isStringColumn",
                "schema" : {
                  "const" : true
                }
              }
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

UIEXT-3099 (WebUI-Migration Binner (Dictionary))

…D columns Also separates String/String-compatible value sorting widgets. AP-23732 (Sorter/Top-K: change default of "comparison" mode for all string-compatible types (except String and RowID) and improve labels/description)

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-15T14:25:01Z

+<?xml version="1.0" encoding="UTF-8"?>
+<config xmlns="http://www.knime.org/2008/09/XMLConfig" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.knime.org/2008/09/XMLConfig http://www.knime.org/XMLConfig_2008_09.xsd" key="test">
+    <entry key="valueColumnPort0" type="xstring" value="Universe_0_0"/>
+    <entry key="lowerBoundColumnPort1" type="xstring" value="Universe_1_0"/>
+    <entry key="upperBoundColumnPort1" type="xstring" value="Universe_1_1"/>
+    <entry key="lowerBoundInclusive" type="xboolean" value="false"/>
+    <entry key="upperBoundInclusive" type="xboolean" value="true"/>
+    <entry key="labelColumnPort1" type="xstring" value="Cluster Membership"/>
+    <entry key="failIfNoRuleMatches" type="xboolean" value="false"/>
+    <entry key="useBinarySearch" type="xboolean" value="false"/>
+</config>


These newly added BinByDictionary snapshot baselines appear unrelated to AP-23732 (Sorter/Top-K string-comparison defaults and labels). If they’re just snapshot-regeneration side effects, they should be moved to a separate PR to keep scope focused and avoid mixing unrelated test-data changes with functional sorting changes.

Copilot · 2026-04-15T14:25:01Z

+{
+  "data" : {
+    "model" : {
+      "valueColumnPort0" : "Universe_0_0",
+      "boundChoices" : {
+        "boundChoice" : "BOTH_BOUNDS",
+        "lowerColumn" : "Universe_1_0",
+        "upperColumn" : "Universe_1_1"
+      },
+      "lowerBoundInclusive" : false,
+      "upperBoundInclusive" : true,
+      "labelColumnPort1" : "Cluster Membership",
+      "noRuleMatchBehavior" : "INSERT_MISSING",
+      "searchPattern" : "LINEAR"
+    }
+  },
+  "schema" : {
+    "type" : "object",
+    "properties" : {
+      "model" : {


These newly added BinByDictionary snapshot baselines appear unrelated to AP-23732 (Sorter/Top-K string-comparison defaults and labels). If they’re just snapshot-regeneration side effects, they should be moved to a separate PR to keep scope focused and avoid mixing unrelated test-data changes with functional sorting changes.

Copilot · 2026-04-15T14:25:02Z

+            @Override
+            public void save(final StringComparisonSetting s, final NodeSettingsWO settings) {
+                if (s.m_isActualStringOrRowId) {
+                    settings.addString(CONFIG_KEY, s.m_stringComparison.name());
+                } else if (s.m_isOtherStringCompatible) {
+                    settings.addString(CONFIG_KEY, s.m_typedStringComparison.toStringComparison().name());
+                }
+                // for non-String-compatible columns, we don't persist anything, because it's not relevant


StringComparisonPersistor.save() decides whether to persist the legacy "stringComparison" key based on UI-only state flags (m_isActualStringOrRowId / m_isOtherStringCompatible). These flags are not persisted and may not be computed in headless settings serialization (e.g., when saving settings without opening the dialog), which can cause the stringComparison setting to be dropped from NodeSettings and change behavior / break backward compatibility. Persist the comparison deterministically (e.g., always write CONFIG_KEY based on a single canonical field, or keep m_stringComparison in sync with m_typedStringComparison so the persistor can always save m_stringComparison, or store an explicit persisted discriminator instead of relying on transient state-provider booleans).

Suggested change

@Override

public void save(final StringComparisonSetting s, final NodeSettingsWO settings) {

if (s.m_isActualStringOrRowId) {

settings.addString(CONFIG_KEY, s.m_stringComparison.name());

} else if (s.m_isOtherStringCompatible) {

settings.addString(CONFIG_KEY, s.m_typedStringComparison.toStringComparison().name());

}

// for non-String-compatible columns, we don't persist anything, because it's not relevant

private static StringComparison getPersistedStringComparison(final StringComparisonSetting s) {

if (s.m_stringComparison != null) {

return s.m_stringComparison;

}

return s.m_typedStringComparison.toStringComparison();

}

@Override

public void save(final StringComparisonSetting s, final NodeSettingsWO settings) {

settings.addString(CONFIG_KEY, getPersistedStringComparison(s).name());

Copilot · 2026-04-15T14:25:02Z

+{
+  "data" : {
+    "model" : {
+      "valueColumnPort0" : "",
+      "boundChoices" : {
+        "boundChoice" : "BOTH_BOUNDS",
+        "lowerColumn" : "",
+        "upperColumn" : ""
+      },
+      "lowerBoundInclusive" : false,
+      "upperBoundInclusive" : true,
+      "labelColumnPort1" : "",
+      "noRuleMatchBehavior" : "INSERT_MISSING",
+      "searchPattern" : "LINEAR"
+    }


These newly added BinByDictionary snapshot baselines appear unrelated to AP-23732 (Sorter/Top-K string-comparison defaults and labels). If they’re just snapshot-regeneration side effects, they should be moved to a separate PR to keep scope focused and avoid mixing unrelated test-data changes with functional sorting changes.

sonarqubecloud · 2026-04-15T15:03:15Z

Quality Gate failed

Failed conditions
78.6% Coverage on New Code (required ≥ 85%)

See analysis details on SonarQube Cloud

enplotz requested a review from a team as a code owner March 27, 2026 14:31

enplotz requested review from Copilot and knime-ghub-bot March 27, 2026 14:31

Copilot started reviewing on behalf of enplotz March 27, 2026 14:31 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

enplotz force-pushed the enh/AP-23732-sorter-node-change-default-of-comparison-mode branch 15 times, most recently from ac7c6c4 to 782d828 Compare March 31, 2026 16:49

LeoWoerteler requested changes Apr 7, 2026

View reviewed changes

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

LeoWoerteler force-pushed the enh/AP-23732-sorter-node-change-default-of-comparison-mode branch 3 times, most recently from d824b0e to 80183f3 Compare April 14, 2026 20:07

LeoWoerteler requested a review from Copilot April 15, 2026 05:04

Copilot started reviewing on behalf of LeoWoerteler April 15, 2026 05:05 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

LeoWoerteler force-pushed the enh/AP-23732-sorter-node-change-default-of-comparison-mode branch from 80183f3 to 1950413 Compare April 15, 2026 08:14

LeoWoerteler requested a review from Copilot April 15, 2026 08:16

Copilot started reviewing on behalf of LeoWoerteler April 15, 2026 08:16 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

Comment thread org.knime.base/src/org/knime/base/node/util/preproc/SortingUtils.java Outdated

LeoWoerteler force-pushed the enh/AP-23732-sorter-node-change-default-of-comparison-mode branch 2 times, most recently from 1917e3c to 45ffaba Compare April 15, 2026 13:21

LeoWoerteler approved these changes Apr 15, 2026

View reviewed changes

LeoWoerteler requested a review from Copilot April 15, 2026 13:31

Copilot started reviewing on behalf of LeoWoerteler April 15, 2026 13:32 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread ...ime.base.tests/src/org/knime/base/node/util/preproc/StringComparisonDefaultProviderTest.java Outdated

enplotz added 2 commits April 15, 2026 16:15

UIEXT-3099: add forgotten test snapshot files

16580f5

UIEXT-3099 (WebUI-Migration Binner (Dictionary))

LeoWoerteler force-pushed the enh/AP-23732-sorter-node-change-default-of-comparison-mode branch from 45ffaba to 04b1b02 Compare April 15, 2026 14:17

LeoWoerteler changed the title ~~AP-23732: Change default for Sorter/Top-K for non-String and non-RowI…~~ AP-23732: Change default for Sorter/Top-K for non-String and non-RowID columns Apr 15, 2026

LeoWoerteler requested a review from Copilot April 15, 2026 14:19

Copilot started reviewing on behalf of LeoWoerteler April 15, 2026 14:19 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Conversation

enplotz commented Mar 27, 2026 • edited by LeoWoerteler Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 15, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

enplotz commented Mar 27, 2026 •

edited by LeoWoerteler

Loading