Skip to content

feat(ui): add prefix-length threshold to tree-view key separator - #6409

Open
Rasu-Dev wants to merge 8 commits into
redis:mainfrom
Rasu-Dev:feat/browser-tree-prefix-length
Open

feat(ui): add prefix-length threshold to tree-view key separator#6409
Rasu-Dev wants to merge 8 commits into
redis:mainfrom
Rasu-Dev:feat/browser-tree-prefix-length

Conversation

@Rasu-Dev

@Rasu-Dev Rasu-Dev commented Aug 12, 2026

Copy link
Copy Markdown

Add a numeric "Ignore separator in first N chars" setting to the browser tree-view settings panel. When set, delimiters within the first N characters of a key name are skipped when building the tree hierarchy.

  • BrowserStorageItem: add treeViewPrefixLength for localStorage persistence
  • StateAppContext.dbConfig: add treeViewDelimiterPrefixLength field
  • appContextSlice: add initial state and setBrowserTreePrefixLength reducer
  • constructKeysToTree: add splitWithPrefixThreshold helper (inside the exported function — required for web worker serialization scope)
  • VirtualTree: accept and forward prefixLength prop into the worker call
  • KeyTree: read treeViewDelimiterPrefixLength from Redux, pass to VirtualTree
  • KeyTreeSettings: add NumericInput field wired to Redux state

Default is 0 (disabled — preserves existing behavior).

What

Added a prefix-length threshold setting to the Browser Tree View key separator. When configured, the first N characters of a key name are excluded from delimiter splitting — so keys like myapp:public:resource where the prefix myapp contains no meaningful separator boundary are grouped correctly.

Changes:
New numeric input "Ignore separator in first N chars" in the Key Tree Settings popover (KeyTreeSettings.tsx)
splitWithPrefixThreshold logic in constructKeysToTree.ts — treats the first N characters as an opaque prefix before applying delimiter splitting
New Redux state field treeViewDelimiterPrefixLength in the dbConfig slice (context.ts, interfaces/app.ts)
New treeViewPrefixLength key in BrowserStorageItem for per-database localStorage persistence (storage.ts)
Default value of 0 means no change to existing behavior

Testing

Manual:
Open Browser → Tree View → Settings (gear icon)
Set a delimiter (e.g. :) and a prefix length (e.g. 5)
Verify keys shorter than N chars are split normally
Verify keys longer than N chars skip delimiter splitting within the first N characters
Verify the value persists across page reloads (stored per-database in localStorage)
Verify resetting the settings panel reverts pending changes
Verify setting prefix length back to 0 restores default behavior

image

Note

Low Risk
UI-only browser tree grouping and settings persistence; default 0 preserves current behavior, with unit tests for the new splitting logic.

Overview
Adds a browser tree-view setting to ignore delimiters in the first N characters of key names, so keys like tenant:app:resource can group under tenant:app instead of splitting at the first :.

Persistence & API: treeViewDelimiterPrefixLength is stored per database (Redux dbConfig, localStorage, and DatabaseSettingsData).

Tree building: constructKeysToTree and VirtualTree take prefixLength and use splitWithPrefixThreshold (duplicated inline in the worker-serialized tree builder). KeyTree passes the setting through, uses the same split when auto-expanding parents for the selected key, and KeyTreeSettings exposes a numeric input (0–512) that dispatches setBrowserTreePrefixLength and resets the tree on apply.

Default 0 keeps prior delimiter-only behavior. Tests cover prefix edge cases; i18n strings added for EN/BG.

Reviewed by Cursor Bugbot for commit 7ad9d6a. Bugbot is set up for automated code reviews on this repo. Configure here.

Add a numeric "Ignore separator in first N chars" setting to the
browser tree-view settings panel. When set, delimiters within the
first N characters of a key name are skipped when building the
tree hierarchy.

This fixes GlobalId keys ({GUID}:{randomId}, ~69 chars) that contain
colons inside the randomId portion — they previously fragmented into
4-5 tree levels. Setting prefix length to 64 collapses them to a
clean 2-level hierarchy.

- BrowserStorageItem: add treeViewPrefixLength for localStorage persistence
- StateAppContext.dbConfig: add treeViewDelimiterPrefixLength field
- appContextSlice: add initial state and setBrowserTreePrefixLength reducer
- constructKeysToTree: add splitWithPrefixThreshold helper (inside
  the exported function — required for web worker serialization scope)
- VirtualTree: accept and forward prefixLength prop into the worker call
- KeyTree: read treeViewDelimiterPrefixLength from Redux, pass to VirtualTree
- KeyTreeSettings: add NumericInput field wired to Redux state

Default is 0 (disabled — preserves existing behavior).
@Rasu-Dev
Rasu-Dev requested a review from a team as a code owner August 12, 2026 15:28
@CLAassistant

CLAassistant commented Aug 12, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7c2274937

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +182 to +183
state.dbConfig.treeViewDelimiterPrefixLength =
payload?.treeViewDelimiterPrefixLength ?? 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Load the prefix setting from its persisted key

After a user applies a non-zero prefix length and reloads or reconnects to the database, this falls back to 0 because setBrowserTreePrefixLength persists the value under BrowserStorageItem.treeViewPrefixLength (treeViewPrefixLength), while setDbConfig reads payload.treeViewDelimiterPrefixLength. The same localStorage object is passed into setDbConfig on instance load, so the saved setting is ignored and the tree reverts to splitting from the first character.

Useful? React with 👍 / 👎.

Comment on lines +194 to +197
<FormField
layout="horizontal"
label={t('browser.tree.settings.prefixLength', 'Ignore separator in first N chars')}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the new label to the locale files

This introduces a new user-facing label through a hardcoded default value, but browser.tree.settings.prefixLength is absent from both redisinsight/ui/src/i18n/locales/en.json and bg.json, so localized builds have no translatable source string for this setting and will fall back to English. Add the key to both locale files instead of relying on the inline default.

AGENTS.md reference: AGENTS.md:L125-L128

Useful? React with 👍 / 👎.

Comment thread redisinsight/ui/src/slices/app/context.ts
Comment thread redisinsight/ui/src/slices/app/context.ts
- Fix treeViewPrefixLength enum value to 'treeViewDelimiterPrefixLength'
  so setDbConfig can hydrate the field correctly from localStorage on reload
- Add @expose() treeViewDelimiterPrefixLength to DatabaseSettingsData DTO
  so the API GET response includes the field instead of stripping it
Comment thread redisinsight/ui/src/pages/browser/components/key-tree/KeyTree.tsx Outdated
…f labels

- Export splitWithPrefixThreshold at module level and fix boundary condition
  from <= to < so keys whose length equals prefixLength are treated as a single
  segment rather than being fully split by the delimiter
- Fix openSelectedKey to use threshold-aware split and reconstruct folder
  fullName paths without trailing delimiter, matching how formatTreeData
  builds them, so selected-key ancestors expand correctly
- Fix shortName in VirtualTree getNodeData to use splitWithPrefixThreshold
  so leaf labels show the full merged token instead of just the trailing fragment
- Add unit tests for splitWithPrefixThreshold and constructKeysToTree with
  prefixLength > 0 covering boundary cases

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24920eb86e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +42 to +46
const nameSplitted = splitWithPrefixThreshold(
name,
delimiterPattern,
prefixLength,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Inline helper before passing to web worker

When the browser tree renders, VirtualTree passes constructKeysToTree to useDisposableWebworker, which creates the Worker from only that function's source; this new call relies on the module-scope splitWithPrefixThreshold, so the worker has no binding for it and throws a ReferenceError for every non-empty key list, even with prefixLength = 0, leaving the tree construction stuck instead of showing keys. Move the helper inside constructKeysToTree or otherwise include it in the serialized worker function.

Useful? React with 👍 / 👎.

Comment thread redisinsight/ui/src/helpers/constructKeysToTree.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d434152e7e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +134 to +138
{ nameString: 'ab:cd', type: KeyTypes.Hash, ttl: -1, size: 0 },
] as unknown as IKeyPropTypes[],
delimiterPattern: ':',
delimiters: [':'],
prefixLength: 10,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fix the failing prefix-length expectation

With this input, splitWithPrefixThreshold takes its name.length < pLength branch for ab:cd and prefixLength: 10, so it performs a normal split into ['ab', 'cd']; constructKeysToTree therefore returns a root folder, not a root leaf. This newly added spec will fail whenever the helper tests run, so either use an equal-length prefix for the single-leaf case or update the expectation to match the intended shorter-than-N behavior.

Useful? React with 👍 / 👎.

Comment thread redisinsight/ui/src/helpers/constructKeysToTree.ts Outdated
…rmat

The openSelectedKey refactor (24920eb) changed parent-path reconstruction
to omit the trailing delimiter, aligning with constructKeysToTree's
`fullName = previousKey + name` output. The test mock's folderFullName
was never updated, causing the auto-expand assertion to look up
`statusOpen['car:']` instead of `statusOpen['car']` and always fail.
@pawelangelow

pawelangelow commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@Rasu-Dev Thank you very much for this contribution, and for working through all of the bot's recommendations.

There’s one thing that’s currently a blocker: the CLA signing. The license bot can’t match the email used in your commits to a GitHub account. Please either add that email to your GitHub account or amend the commits to use an email that’s already associated with it, then re-run the check.

On the code itself, there are a few things that could still be addressed:

  1. Test saving and restoring - Please cover the round trip for the new setting. This path has broken before, so it would be good to have explicit coverage.

  2. Test the settings panel - Please add a case for the new input alongside the existing panel tests.

  3. Remove the duplicated helper - Could you keep splitWithPrefixThreshold as a single declaration inside constructKeysToTree and remove the exported copy?

    To clarify, having the helper inside constructKeysToTree is intentional. constructKeysToTree gets stringified into a Web Worker (useWebworkers.ts builds the worker from the function’s own source), so it can’t reference anything at module scope. An exported helper would result in a ReferenceError inside the worker. Because that happens off the main thread, the tree could silently retain its previous contents without any visible error for the user.

    The issue is the second, exported copy: the unit tests exercise that copy, while the application actually runs the inlined one. Nothing guarantees that the two implementations stay in sync, and if they drift, the tests won’t catch it.

    We don’t need to lose the coverage. Could you move the existing splitWithPrefixThreshold cases to tests for constructKeysToTree itself, asserting the resulting folder structure for the same scenarios:

    • threshold of 0
    • threshold longer than the key
    • delimiter starting exactly at the threshold
    • multiple delimiters

    That preserves the same guarantees while leaving us with a single copy of the logic — the one that actually runs in the application.

  4. Add a maximum and some help text to the field - A sufficiently large value effectively flattens the entire tree to one level, and the label alone doesn’t make that behavior clear.

Thanks again for the contribution!

@Rasu-Dev
Rasu-Dev force-pushed the feat/browser-tree-prefix-length branch from e267d6b to 8eec72c Compare August 13, 2026 07:38

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 8eec72c. Configure here.

Comment thread redisinsight/ui/src/pages/browser/components/virtual-tree/VirtualTree.tsx Outdated
Add browser.tree.settings.prefixLength / prefixLengthHint to the en and
bg locales, and deduplicate splitWithPrefixThreshold into a shared
helper (uiSrc/helpers) imported by KeyTree and VirtualTree.
constructKeysToTree keeps its inline copy, which the Web Worker
stringification requires.
@Rasu-Dev
Rasu-Dev force-pushed the feat/browser-tree-prefix-length branch from 246aaa7 to 139734a Compare August 13, 2026 10:30
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.

3 participants