feat(ui): add prefix-length threshold to tree-view key separator - #6409
feat(ui): add prefix-length threshold to tree-view key separator#6409Rasu-Dev wants to merge 9 commits into
Conversation
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).
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 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".
| state.dbConfig.treeViewDelimiterPrefixLength = | ||
| payload?.treeViewDelimiterPrefixLength ?? 0 |
There was a problem hiding this comment.
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 👍 / 👎.
| <FormField | ||
| layout="horizontal" | ||
| label={t('browser.tree.settings.prefixLength', 'Ignore separator in first N chars')} | ||
| > |
There was a problem hiding this comment.
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 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- 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
…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
There was a problem hiding this comment.
💡 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".
| const nameSplitted = splitWithPrefixThreshold( | ||
| name, | ||
| delimiterPattern, | ||
| prefixLength, | ||
| ) |
There was a problem hiding this comment.
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 👍 / 👎.
…or Web Worker capture
There was a problem hiding this comment.
💡 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".
| { nameString: 'ab:cd', type: KeyTypes.Hash, ttl: -1, size: 0 }, | ||
| ] as unknown as IKeyPropTypes[], | ||
| delimiterPattern: ':', | ||
| delimiters: [':'], | ||
| prefixLength: 10, |
There was a problem hiding this comment.
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 👍 / 👎.
…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.
|
@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:
Thanks again for the contribution! |
e267d6b to
8eec72c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 8eec72c. Configure here.
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.
246aaa7 to
139734a
Compare
|
@pawelangelow Could you please review these changes? |
|
@Rasu-Dev Thanks for the nudge, and sorry for the delay. The way you've split the helper is fine as it stands: keep the shared module, and the inline copy inside constructKeysToTree for the Web Worker, with the comments pointing at each other. Please ignore my earlier request to collapse them into one. One thing left. The save-then-restore test: the two new cases assert Redux state only, so they would pass even with the storage field mismatch that was fixed earlier here. Asserting the field name the storage write receives, or restoring from what actually landed in storage, would catch that. With that in, this is good from my side. |
…e test The setBrowserTreePrefixLength case asserted Redux state only, so it would pass even if the BrowserStorageItem.treeViewPrefixLength enum value stopped matching the field setDbConfig hydrates from. Now the test reads back what actually landed in localStorage, asserts the literal field name, and restores state from the stored object.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 176494f1b4
ℹ️ 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".
| const prefix = name.substring(0, pLength) | ||
| const rest = name.substring(pLength) |
There was a problem hiding this comment.
For keys containing non-BMP characters, substring counts UTF-16 code units rather than the characters represented by the UI's “first N chars” setting. For example, with 😀:user:item and prefixLength = 2, users count the emoji and colon as two characters and expect that colon to be ignored, but substring(0, 2) contains only the emoji, so the colon remains in rest and incorrectly creates a folder boundary. Use a code-point-aware representation for the split, and apply the same correction to the inline worker copy.
Useful? React with 👍 / 👎.
|
@pawelangelow I’ve made the suggested changes to the save-then-restore test. Could you please check it again? |
3 participants
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.
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
Note
Low Risk
Browser-only display and grouping preference with default 0; no auth, data mutation, or server execution paths beyond optional settings field exposure.
Overview
Adds a per-database “Ignore separator in first N chars” control in Browser tree-view settings so delimiters inside the first N characters of a key name are skipped when building the folder hierarchy (default 0 keeps current behavior).
Persistence & API:
treeViewDelimiterPrefixLengthis stored in ReduxdbConfig, written to per-database localStorage, and exposed onDatabaseSettingsDatafor server-side settings sync.Tree building:
constructKeysToTreeand the web worker path usesplitWithPrefixThreshold(shared helper plus an inline copy for worker serialization).VirtualTreeforwardsprefixLength; folder short names and auto-expand of parents for the selected key use the same splitting rules, with parentfullNamereconstruction aligned to tree formatting (fixes expectations such as foldercarvscar:in tests).UI:
KeyTreeSettingsadds a bounded numeric field; Apply dispatchessetBrowserTreePrefixLengthand resets the tree. EN/BG copy documents the hint.Reviewed by Cursor Bugbot for commit 176494f. Bugbot is set up for automated code reviews on this repo. Configure here.