Skip to content

Fix to match non-english titles in document-list filter#311

Merged
hackerwins merged 1 commit into
mainfrom
fix/search-docs
May 29, 2026
Merged

Fix to match non-english titles in document-list filter#311
hackerwins merged 1 commit into
mainfrom
fix/search-docs

Conversation

@ggyuchive

@ggyuchive ggyuchive commented May 29, 2026 •
edited by hackerwins
Loading

Copy link
Copy Markdown
Member

Summary

TanStack Table's default includesString filter compares Unicode code points as-is. Korean text from sources that emit decomposed jamo (macOS file system, some pasted content) does not match a query typed through an IME that emits NFC-precomposed syllables (the common case) — so a query of "이" missed titles like "이력서" when the stored title was NFD. Apply NFC normalization on both sides of the includes check in a custom filterFn on the title column.

Why

Linked Issues

Fixes #

Author checklist

  • I searched existing issues and PRs and confirmed this is not a duplicate.
  • I read every line of this diff and can explain why each change is necessary.
  • Changes follow the conventions in the touched packages; any deviations are explained in Why above.
  • If AI tools assisted with this PR, I noted where in Notes for Reviewers below.

Verification

CI automatically posts a verification summary comment on this PR with
per-lane results for both verify:self and verify:integration.

  • verify:self — CI comment shows ✅
  • verify:integration — CI comment shows ✅ (or explicit skip reason below)

Skip reason (if applicable):

Risk Assessment

  • User-facing risk:
  • Data/security risk:
  • Rollback plan:

Notes for Reviewers

  • UI changes (screenshots/gifs if applicable):
  • Follow-up work (if any):

Summary by CodeRabbit

  • Bug Fixes
    • Improved the document title filter to provide more reliable matching, now supporting case-insensitive searches and better handling of special characters.

Review Change Stack

@ggyuchive
Match Korean titles in document-list filter via NFC normalize
67732d8 
TanStack Table's default includesString filter compares Unicode code
points as-is. Korean text from sources that emit decomposed jamo
(macOS file system, some pasted content) does not match a query typed
through an IME that emits NFC-precomposed syllables (the common case)
— so a query of "이" missed titles like "이력서" when the stored
title was NFD. Apply NFC normalization on both sides of the includes
check in a custom filterFn on the title column.
@coderabbitai

coderabbitai Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR modifies the document list's title column filter to normalize Unicode characters and perform case-insensitive substring matching. A custom filterFn is added that normalizes both the row value and filter input using NFC normalization, lowercases both strings, and then performs an includes comparison.

Changes

Title Filter Enhancement

Layer / File(s) Summary
Normalized title column filter
packages/frontend/src/app/documents/document-list.tsx		 Title column filter function applies Unicode NFC normalization and lowercasing to both cell values and filter input for case-insensitive substring matching.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A filter that's kinder to scripts and to text,
With Unicode normalized—no special chars vexed,
Case doesn't matter, just substring it finds,
Documents searchable for all kinds of minds! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing non-English (specifically Unicode normalization) title matching in the document-list filter, which is the core purpose of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/search-docs

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 and usage tips.

@github-actions

github-actions Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Verification: verify:self

Result: ✅ PASS in 228.2s

Lane Status Duration
tokens:build ✅ pass 2.2s
sheets:build ✅ pass 12.5s
docs:build ✅ pass 11.6s
slides:build ✅ pass 13.5s
verify:fast ✅ pass 145.8s
frontend:build ✅ pass 17.3s
verify:frontend:chunks ✅ pass 0.3s
backend:build ✅ pass 4.7s
cli:build ✅ pass 2.0s
verify:entropy ✅ pass 18.2s

Verification: verify:integration

Result: ✅ PASS

@codecov

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@hackerwins
hackerwins merged commit cfae858 into main May 29, 2026
4 checks passed
@hackerwins
hackerwins deleted the fix/search-docs branch May 29, 2026 16:23
hackerwins added a commit that referenced this pull request May 29, 2026
@hackerwins
Merge main into feat/docs-font-controls
6fa02be 
Pull in PR #309 (slides format options right panel) and #311 (Korean
NFC normalize in document-list filter). Only conflict was the archived
tasks index, regenerated via pnpm tasks:index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ggyuchive @hackerwins