Skip to content

233 add reductselect to builder - #246

Open
Marrii00 wants to merge 26 commits into
mainfrom
233-add-reductselect-to-builder
Open

233 add reductselect to builder#246
Marrii00 wants to merge 26 commits into
mainfrom
233-add-reductselect-to-builder

Conversation

@Marrii00

@Marrii00 Marrii00 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Closes #233

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • CHANGELOG.md has been updated (for bug fixes / features / docs)

What kind of change does this PR introduce?

Feature.

What was changed?

Added a "Process (Select)" step to the Data Explorer's Conditional Query builder, letting users configure a ReductSelect transform (SQL expression, output format, and label mapping) visually instead of hand-writing the #ext JSON.

  • New SelectStepEditor component with a unified "+ Add option" dropdown (Format / Protobuf / Export / As label), mirroring the existing ReductROS step's conventions: options stay visible but greyed out with a tooltip when unavailable instead of being hidden, and row-list options (Protobuf fields, As label) reuse the same button to add both the first row and subsequent ones.
  • CSV, JSON, and Parquet are grouped under a single "Format" option with a segmented control to switch between them, since they're mutually exclusive alternatives of the same slot; Protobuf (with a message name/schema pair and a dedicated 3-column field list for column/id/type) and Export (format/rows/duration) are separate, independently addable sections.
  • Generalized the internal TransformStepEntry type into a discriminated union (ros | select) so the builder can host either transform kind, with all existing ReductROS behavior and tests unchanged.
  • Extracted the shared key/value row-list UI (KeyValueRowList) out of the ReductROS editor so both transform kinds reuse it.
  • Fixed a pre-existing responsiveness bug where the Label filter's "Where" row could overflow its card on narrow viewports.
  • Various row-alignment and width tweaks across the builder so remove buttons and input columns line up consistently between sections.

Related issues

Closes #233.

Does this PR introduce a breaking change?

No. This only adds a new, optional step type to the builder; existing ReductROS steps and hand-written JSON queries are unaffected.

Other information:

None.

Copilot AI lite review requested due to automatic review settings September 4, 2026 12:40
@Marrii00 Marrii00 linked an issue Sep 4, 2026 that may be closed by this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Select payload parsing/building currently permits invalid/unsupported format combinations and protobuf field ids, which can produce inconsistent UI states and invalid #ext.select output.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new “Process (Select)” transform step to the Conditional Query builder, enabling visual configuration of a ReductSelect #ext.select payload alongside the existing ROS transform.

Changes:

  • Introduces Select transform state, mutators, payload build/parse logic, and test coverage in transformStepBuilder.
  • Adds SelectStepEditor UI (plus reusable row-list components) and wires it into the builder/block list with new tests.
  • Tweaks row layout constants and fixes a narrow-viewport overflow in the label filter editor.
File summaries
File Description
src/Helpers/transformStepBuilder.ts Adds Select transform model/mutators plus build/parse handling for #ext.select.
src/Helpers/transformStepBuilder.test.ts Expands unit tests to cover Select transform behavior and ROS/Select exclusivity.
src/Components/QueryConditionBuilder/TransformStepEditor.tsx Extracts shared key/value row-list UI and improves export row wrapping.
src/Components/QueryConditionBuilder/stepRowLayout.ts Adjusts shared widths and adds protobuf/export-specific width constants.
src/Components/QueryConditionBuilder/SelectStepEditor.tsx New editor UI for Select transform configuration (SQL, format, protobuf, export, as_label).
src/Components/QueryConditionBuilder/SelectStepEditor.test.tsx New component tests for SelectStepEditor interactions and menu behavior.
src/Components/QueryConditionBuilder/QueryConditionBuilder.tsx Wires Select transform creation and mutation handlers into builder state updates.
src/Components/QueryConditionBuilder/QueryConditionBuilder.test.tsx Adds integration tests for Process (Select) block behavior and #ext output.
src/Components/QueryConditionBuilder/QueryBlockList.tsx Adds Process (Select) block rendering and add-step menu gating vs ROS.
src/Components/QueryConditionBuilder/QueryBlockList.test.tsx Updates/adds tests for new Process (Select) menu and block behavior.
src/Components/QueryConditionBuilder/ProtobufFieldRowList.tsx New UI for editing protobuf field mapping rows (column/id/type).
src/Components/QueryConditionBuilder/LabelConditionEditor.tsx Fixes narrow-viewport overflow by allowing wrapping and simplifying sizing.
src/Components/QueryConditionBuilder/KeyValueRowList.tsx New shared key/value row-list component reused by ROS + Select editors.
Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Components/QueryConditionBuilder/ProtobufFieldRowList.tsx
Comment thread src/Components/QueryConditionBuilder/SelectStepEditor.tsx Outdated
Comment thread src/Helpers/transformStepBuilder.ts Outdated
Comment thread src/Helpers/transformStepBuilder.ts
Copilot stopped work on behalf of Marrii00 due to an error September 7, 2026 13:50

@AnthonyCvn AnthonyCvn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

move steps to components folder

Comment thread src/Components/Steps/SqlInput.tsx Outdated
interface Window {
__sqlCompletionProviderDisposable?: IDisposable;
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it for vitest ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not related to vitest. It's there to handle Vite's hot reload in dev: without it, every time a file is saved, the module reloads and re-registers the autocomplete provider without removing the old one, so suggestions end up appearing duplicated. This bit of code just cleans up the old provider before registering a new one. In production this never happens, since the module only loads once.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah but it's not best practice in React. Please check implementation of JsonQueryEditor. It is the same, also with auto-complete.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done, merged them into one QueryEditor component, used for both JSON and SQL now.

<Button
aria-label={onlyRow ? sectionRemoveLabel : removeLabel}
type="text"
icon={<CloseOutlined style={{ transform: "scale(0.65)" }} />}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

to check if we need a transform or fixed size here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it was already there since #243 (issue #238), in TransformStepEditor.tsx's icons. Since we're already moving/touching these files here, I fixed it everywhere it showed up (9 spots across 6 files) instead of leaving it inconsistent, and centralized the value into a new ROW_ICON_FONT_SIZE constant in stepRowLayout.ts, matching how the other shared sizing constants (ROW_LABEL_WIDTH, ROW_INPUT_WIDTH, etc.) already work there.

@Marrii00
Marrii00 requested a review from AnthonyCvn September 8, 2026 12:43

@AnthonyCvn AnthonyCvn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Combining ReductSelect with ReductROS should be possible from the builder:

{
  "$each_t": "$__interval",
  "#ext": {
    "ros": {
      "extract": {}
    },
    "select": {
      "sql": "SELECT * FROM ENTRY()"
    }
  }
}

Comment thread package.json Outdated
"@dnd-kit/utilities": "^3.2.2",
"@monaco-editor/react": "^4.7.0",
"@reductstore/reduct-query-monaco": "^1.0.1",
"@reductstore/reduct-query-monaco": "github:reductstore/reduct-query-monaco#feat/sql-completion-provider",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should test locally with npm link without touching package.json. We can update the version here once reduct-query-monaco is published.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

reverted the dependency

Comment thread src/Components/Steps/SqlInput.tsx Outdated
SELECT * FROM ENTRY()
</div>
)}
<Editor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to reuse the JsonQueryEditor and make it universal as queryEditor so it can be used for JSON and SQL? it has more options to resize, format and so on which is missing here and would be great to have as an editor.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes good idea, i'll implement it

@AnthonyCvn

Copy link
Copy Markdown
Member

Did you understand why i get "column_0" there ?

Screenshot 2026-09-08 at 16 32 53

@Marrii00

Marrii00 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Did you understand why i get "column_0" there ?

Screenshot 2026-09-08 at 16 32 53

I implemented it because the ReductSelect docs say CSV files without a header get their columns named column_0, column_1, etc. It's specific to CSV, doesn't really apply to other formats. I show it as a reminder of that naming convention, not a real detected column, since the provider has no access to the actual data behind a query.

I wanted to stay close to what the docs describe and figured it'd be a useful hint, but maybe I'm wrong about that. If it's more confusing than helpful, I can just remove it from the suggestions?

@Marrii00
Marrii00 requested a review from AnthonyCvn September 9, 2026 12:41

@AnthonyCvn AnthonyCvn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we also remove the "shadow" below an element when it moves ?

And unrelated but the button like we used to have was somehow nicer on the left =) if can move it back on this PR it would be great, otherwise next one.
Image

@@ -85,7 +118,7 @@ export default function QueryBlockList({
blockOrder,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Too many pops, use composition instead.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added a new file (builderReducer.ts) centralizing all state updates through a single useReducer.

onChangeSelectExport,
onAddAsLabelRow,
onChangeAsLabelRow,
onRemoveAsLabelRow,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here you can use useReducer hook from react to dispatch your changes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as for QueryBlockList.tsx. Added a new file (builderReducer.ts) centralizing all state updates through a single useReducer.

updateSelectExport,
updateSql,
updateTopic,
} from "../../Helpers/transformStepBuilder";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can also use the useReducer hook for the QueryConditionBuilder

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as for QueryBlockList.tsx. Added a new file (builderReducer.ts) centralizing all state updates through a single useReducer.

);
}

interface TransformStepEditorProps {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TransformStepEditor can also use composition + useReducer hook

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done, TransformStepEditor now takes a single dispatch prop instead of the individual callbacks

@Marrii00
Marrii00 requested a review from AnthonyCvn September 10, 2026 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ReductSelect to builder

3 participants