Skip to content

fix(llc): normalize strings when comparing ComparableField - #2809

Merged
xsahil03x merged 2 commits into
masterfrom
bug/2601_case_insensitive_diacritic_sort
Jul 14, 2026
Merged

fix(llc): normalize strings when comparing ComparableField#2809
xsahil03x merged 2 commits into
masterfrom
bug/2601_case_insensitive_diacritic_sort

Conversation

@xsahil03x

@xsahil03x xsahil03x commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #2601 — client-side SortOption on name (and any other string field) was returning results in a different order than the server because ComparableField used a raw codepoint compare. Lowercase names and non-ASCII names (Polish Ł, Norwegian Ø, etc.) got pushed to the end instead of sorting alphabetically.

Root cause: ComparableField.compareTo's String branch was a.compareTo(b), i.e. byte-order on UTF-16 code units.

Fix: route string comparisons through a new normalizeStringForSort helper that mirrors the backend's NormalizeName pipeline:

  1. Fold Latin diacritics and ligatures (é → e, Ł → l, Æ → ae) via package:diacritic.
  2. Preserve Japanese kana, Thai, and Vietnamese-specific runes unchanged (matches backend's Normalize early-exit — critical for Japanese combining sound marks in the Hiragana range).
  3. Lowercase.
  4. Trim leading/trailing ASCII apostrophes (U+0027) then whitespace.

Backend has no tie-breaker for sort: name, so we don't either — equal-normalized values compare as 0 and Dart's stable List.sort preserves input order.

Notes

  • SortOption's comparator parameter remains the escape hatch for callers who need raw codepoint or locale-aware behavior.
  • Known deferred divergences (documented in the normalizer file): CJK/Thai word segmentation (backend uses Kagome + TWC dictionaries — no viable pure-Dart port), emoji-to-text token replacement, extended Unicode Mn categories.
  • Adds diacritic: ^0.1.6 to stream_chat (already used by stream_chat_flutter).

Test plan

  • Added packages/stream_chat/test/src/core/util/string_sort_normalizer_test.dart — ports 10 applicable cases from backend's TestNormalizeName + coverage for Vietnamese preservation, ligature folding, combining marks, apostrophe/whitespace trimming, and Japanese combining sound marks (U+3099 / U+309A).
  • Added regression test in comparable_field_test.dart that sorts the exact scenario from the bug report (Zara, jhon, Łukasz, Øystein, Adam, Marek) and verifies the expected alphabetical order.
  • Full stream_chat suite (1551 tests) passes.
  • dart analyze --fatal-infos . clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved client-side sorting of names and other text fields.
    • Sorting is now case-insensitive and handles accented characters and ligatures more consistently.
    • Lowercase and non-ASCII names are no longer incorrectly placed at the end of sorted lists.
  • Documentation

    • Clarified default text-sorting behavior and how to provide custom comparison rules.

Client-side sorts using `SortOption` on string fields no longer push
lowercase names or non-ASCII names to the end of the list — the comparator
now folds diacritics/ligatures and ignores case, mirroring the backend's
`NormalizeName` pipeline (fold Latin diacritics, preserve Japanese/Thai/
Vietnamese-specific runes, lowercase, trim ASCII apostrophes and
whitespace).

Fixes #2601.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

ComparableField now compares normalized string keys that fold case, diacritics, and ligatures while preserving selected scripts and Vietnamese characters. A reusable normalizer, dependency, documentation, changelog entry, and regression tests were added.

Changes

Server-parity string sorting

Layer / File(s) Summary
String sort normalization
packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart, packages/stream_chat/pubspec.yaml
Adds normalization for case folding, diacritics, ligatures, script preservation, Vietnamese-specific characters, and ASCII apostrophe trimming.
ComparableField integration
packages/stream_chat/lib/src/core/models/comparable_field.dart, packages/stream_chat/lib/src/core/api/sort_order.dart, packages/stream_chat/CHANGELOG.md
Routes string comparisons through normalized keys and documents the default comparison behavior and comparator override.
Normalization and comparison tests
packages/stream_chat/test/src/core/models/comparable_field_test.dart, packages/stream_chat/test/src/core/util/string_sort_normalizer_test.dart
Covers normalized ordering, equality, Unicode handling, script preservation, and trimming behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: normalizing ComparableField string comparisons.
Linked Issues check ✅ Passed The changes address #2601 by replacing codepoint comparison with normalized, case-insensitive string sorting and matching tests.
Out of Scope Changes check ✅ Passed The added dependency, normalization utility, docs, and tests all support the string-sorting fix and appear in scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bug/2601_case_insensitive_diacritic_sort

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.27%. Comparing base (550f071) to head (92c112f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2809      +/-   ##
==========================================
+ Coverage   71.24%   71.27%   +0.03%     
==========================================
  Files         429      430       +1     
  Lines       26893    26925      +32     
==========================================
+ Hits        19159    19191      +32     
  Misses       7734     7734              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (2)
packages/stream_chat/lib/src/core/models/comparable_field.dart (1)

42-47: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider caching the normalized key instead of recomputing it on every comparison.

_compareStrings normalizes both operands every time compareTo runs. During a full sort, this repeats normalization for the same value across O(n log n) comparisons instead of computing it once per element. For typical name-length strings and moderate list sizes this is unlikely to matter, but if ComparableField is ever used to sort large lists repeatedly, caching the normalized key at fromValue construction (only for the String case) would avoid the redundant work.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat/lib/src/core/models/comparable_field.dart` around lines
42 - 47, The _compareStrings method recomputes normalized sort keys for every
comparison. Cache the normalized string when ComparableField is created through
fromValue for String inputs, then have _compareStrings reuse those cached keys
while preserving current comparison behavior for non-String values.
packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart (1)

49-56: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Per-rune calls to removeDiacritics re-allocate/lookup for every character.

_foldRune invokes removeDiacritics(String.fromCharCode(rune)) once per rune instead of batching the fold-eligible runes into a single call. Functionally equivalent (the underlying table is a per-code-unit map), but it's more allocation-heavy than necessary for longer strings.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart` around
lines 49 - 56, Update the string normalization flow around _foldRune to batch
fold-eligible runes into a single removeDiacritics call rather than invoking it
once per rune. Preserve direct passthrough behavior for ASCII, Japanese, Thai,
and Vietnamese-specific runes, while accumulating other runes and applying the
existing diacritic removal in one operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/stream_chat/lib/src/core/models/comparable_field.dart`:
- Around line 42-47: The _compareStrings method recomputes normalized sort keys
for every comparison. Cache the normalized string when ComparableField is
created through fromValue for String inputs, then have _compareStrings reuse
those cached keys while preserving current comparison behavior for non-String
values.

In `@packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart`:
- Around line 49-56: Update the string normalization flow around _foldRune to
batch fold-eligible runes into a single removeDiacritics call rather than
invoking it once per rune. Preserve direct passthrough behavior for ASCII,
Japanese, Thai, and Vietnamese-specific runes, while accumulating other runes
and applying the existing diacritic removal in one operation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 529603a9-f582-46d4-99b0-4282af4be337

📥 Commits

Reviewing files that changed from the base of the PR and between 249a92c and 6c910aa.

📒 Files selected for processing (7)
  • packages/stream_chat/CHANGELOG.md
  • packages/stream_chat/lib/src/core/api/sort_order.dart
  • packages/stream_chat/lib/src/core/models/comparable_field.dart
  • packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart
  • packages/stream_chat/pubspec.yaml
  • packages/stream_chat/test/src/core/models/comparable_field_test.dart
  • packages/stream_chat/test/src/core/util/string_sort_normalizer_test.dart

@xsahil03x

Copy link
Copy Markdown
Member Author

The build (android) / build (ios) failures on this PR are not caused by these changes — they're a fresh upstream firebase_messaging publish issue.

Root causefirebase/flutterfire#18438 (opened 2026-07-13, currently open):

firebase_messaging 16.4.2 and firebase_crashlytics 5.2.5 were published referencing a FirebasePlugin base class that doesn't exist in any published firebase_core_platform_interface. Latest published is firebase_core_platform_interface 7.1.0, which only exports the old FirebasePluginPlatform. Any resolve that lands on firebase_messaging 16.4.2 fails at build time with:

firebase_messaging-16.4.2/lib/src/messaging.dart:11:33: Error: Type 'FirebasePlugin' not found.
firebase_messaging-16.4.2/lib/src/messaging.dart:21:36: Error: The getter 'pluginConstants' isn't defined for the type 'FirebaseMessaging'.

Same failure exists on plain master — CI for master's HEAD commit (249a92c45, distribute_internal) fails with identical errors. This PR's diff touches only packages/stream_chat/** (pure Dart LLC) — zero changes to sample_app/, melos.yaml, or any Firebase-related config.

Plan — wait for Firebase to either publish the missing firebase_core_platform_interface release that defines FirebasePlugin or ship a firebase_messaging 16.4.3 that reverts the rename. The stream_chat unit tests, analyze, format, and codecov checks are all green.

Comment thread packages/stream_chat/lib/src/core/models/comparable_field.dart
Comment thread packages/stream_chat/lib/src/core/util/string_sort_normalizer.dart
@xsahil03x
xsahil03x merged commit 0f7f1b6 into master Jul 14, 2026
24 checks passed
@xsahil03x
xsahil03x deleted the bug/2601_case_insensitive_diacritic_sort branch July 14, 2026 11:35
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.

ComparableField String sorting overrides correct backend order with case-sensitive codepoint comparison

2 participants