fix: display same key with multiple data types in filter suggestions by enhancing the deduping logic

[ahmadshaheer]

Summary

Related Issues / PR's

#7205

Screenshots

Before:

After:

Affected Areas and Manually Tested Areas

• query builder filter auto-complete

---

Important

Enhances deduplication logic in useOptions to consider dataType, preventing incorrect deduplication of filter suggestions.

• Behavior:
  • Enhances deduplication logic in useOptions in useOptions.ts to consider dataType when filtering options.
  • Handles case sensitivity in dataType comparison to prevent incorrect deduplication.
• Affected Areas:
  • Impacts query builder filter auto-complete suggestions.

^{This description was created by for d094c22. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 8908422 in 28 seconds

More details

• Looked at 18 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. frontend/src/hooks/queryBuilder/useOptions.ts:172

• Draft comment:
  Enhancement looks good: extending the deduplication to include dataType with trim and lowercase for consistency. Consider using a fallback like (o.dataType || '') to handle null/undefined more explicitly.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

2. frontend/src/hooks/queryBuilder/useOptions.ts:174

• Draft comment:
  Enhanced deduplication now includes a normalized dataType check, which appears to fix the multiple data type issue. Consider extracting the normalization (trim & toLowerCase) into a helper function if this logic is reused elsewhere to improve maintainability.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

Workflow ID: wflow_pccjTFY22nC0TsQj

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[SagarRajput-7]

reran the failing tests, if still fails, then try rebasing

[SagarRajput-7]

Accidentally closed this PR

[ellipsis-dev]

👍 Looks good to me! Incremental review on d094c22 in 2 minutes and 32 seconds

More details

• Looked at 14 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. frontend/src/hooks/queryBuilder/useOptions.ts:177

• Draft comment:
  Removing the .trim() calls may lead to deduplication issues if dataType values contain leading or trailing whitespace. Ensure that this change is intentional.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  The dataType field comes from BaseAutocompleteData which is likely a controlled API response. The trim() was probably unnecessary defensive programming. The comment is speculative ("may lead to") and asks for confirmation ("ensure that"). It's not pointing out a definite issue.
  I could be wrong about the source of dataType values - there could be user input or other sources that introduce whitespace. The original author may have added trim() for a specific reason.
  Even if there was whitespace, having duplicate options with only whitespace differences would be a minor issue at worst. The core functionality of option matching still works with toLowerCase().
  Delete this comment as it's speculative, asks for confirmation, and doesn't identify a clear issue that needs fixing.

2. frontend/src/hooks/queryBuilder/useOptions.ts:177

• Draft comment:
  Was the removal of trim() on dataType intentional? Removing trim means that values with extra spaces won’t be normalized, which could lead to duplicates being considered distinct. If this is the desired behavior, please update the inline comment to explain why.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  The comment raises a valid point about potential duplicates with whitespace, but several factors make me lean towards removing it: 1) dataType is likely an internal type identifier that wouldn't contain arbitrary whitespace 2) The code is just comparing values, not storing them 3) The comment asks for confirmation of intention rather than stating a clear issue 4) The original comment only mentioned case sensitivity, suggesting whitespace wasn't a key concern
  I might be underestimating the importance of whitespace normalization in dataType values. There could be upstream code that introduces whitespace that I'm not aware of.
  While whitespace in dataTypes is possible, the comment violates the rule about asking for confirmation of intention. If whitespace was critical, it would likely have been mentioned in the original comment.
  Delete the comment as it primarily asks for confirmation of intention rather than pointing out a clear issue, and the whitespace concern seems speculative without strong evidence.

Workflow ID: wflow_mDqepGBegxKTMjNK

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.