Add live duplicate check to the new-entry dialog - #2433
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spec-cited review against EntrySearchService/SqlHelpers found the client fold diverged: host-locale lowercasing, unconditional accent-stripping (backend keeps diacritics significant when the query has them), and no morph-token handling (typed "-aji" vs suffix entry "aji" missed exact). Also ranks each search by the writing system it was typed in, fixes the mount-time "Checking" flash, singularizes the one-match banner, and closes a pre-existing double-Enter double-create in the dialog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- match rows expand to reveal Add sense / Go to entry; DictionaryEntry gains an inline mode for the compact collapsed rows - classification: attribute same-word matches to the field that hit (Same headword vs Same lexeme form), rank similar words closest-in-length first, drop cross-field coincidences instead of a vague 'related' kind - classify in a $derived so the lazy morph-types resource warms at mount and re-classifies without re-searching - dedupe queries per field kind: the same text typed as lexeme and gloss keeps its gloss query (same-meaning matches were lost) - share the banner message/tint/Enter-trap between the strip and the extracted DuplicateSummaryPill - scope backend-parity doc claims to the CRDT FTS path; drop unverified FLEx attribution - adapt the Playwright suite to the new interaction model and cover the jump pill (7 tests); pin length-delta boundary, gloss containment direction, and closest-form ranking in unit tests (33; 8/8 targeted mutants killed) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…icates # Conflicts: # frontend/viewer/src/locales/en.po # frontend/viewer/src/locales/es.po # frontend/viewer/src/locales/fr.po # frontend/viewer/src/locales/id.po # frontend/viewer/src/locales/ko.po # frontend/viewer/src/locales/ms.po # frontend/viewer/src/locales/sw.po # frontend/viewer/src/locales/vi.po
…etection-review-ny053c
…fill test gaps - Move new-entry-duplicates.test.ts to tests/ui/ and port it to the DemoProjectPage page object introduced by the E2E restructure (#1866) - Cap the jump pill at 32rem so long headword previews don't stretch it across the whole dialog - New tests: trapEnter unit test, Enter-inside-strip must not create the entry, pill dismiss persists, add-sense must not also create an entry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R49zrCE7hPgm9QJ5bfugaV
A failed searchEntries call previously vanished into the unread resource.error — no toast, no status line, the widget just went blank. Show an inline status line instead of a toast: the search re-fires per typing pause, and a toast per failure would bury the dialog. Show-more now uses $plural so translators get per-language plural forms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R49zrCE7hPgm9QJ5bfugaV
This reverts commit d267bd1.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesDuplicate entry detection
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
…etection-review-ny053c
…ntry-detection-sync-1bcb1f # Conflicts: # frontend/viewer/src/locales/en.po # frontend/viewer/src/locales/es.po # frontend/viewer/src/locales/fr.po # frontend/viewer/src/locales/id.po # frontend/viewer/src/locales/ko.po # frontend/viewer/src/locales/ms.po # frontend/viewer/src/locales/sw.po # frontend/viewer/src/locales/vi.po
Always send the backend search with diacritics stripped (it only matches accent-insensitively for a diacritic-free query), then classify an exact diacritic match as same-word and an accent-only difference as similar-word. Classification now runs against the displayed headword from the writing-system service, dropping the client-side morph-token stripping. Extract the duplicate pill/widget wiring into DuplicateCheckSection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The entries for "Meaning added to {0}" and "This entry may already exist" were extracted without a msgstr line in all seven non-English locales, leaving the catalogs malformed. Add the empty msgstr so they parse as normal untranslated entries.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
expand() sets userToggled=true, which recomputed the `expanded` $derived back to (hasExactWordMatch && !userToggled) = false — so clicking the jump pill closed the strip it was meant to open. Restore `expanded` to $state with an effect for the exact-match auto-open. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0940483 to
e11f04a
Compare
The empty-input test in the normalizeForExactCompare block called normalizeForLooseCompare, so exact-compare's handling of a missing value was never covered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.ts (1)
82-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant trim/coalesce.
normalizeForExactComparealready trims and always returns a string, so.trim() ?? ''on line 83 is dead. Minor cleanup.♻️ Proposed tidy
- const decomposed = normalizeForExactCompare(value).trim() ?? ''; - return decomposed.toLowerCase() + return normalizeForExactCompare(value).toLowerCase() // remove diacritics .replace(/\p{Mn}/gu, '');🤖 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 `@frontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.ts` around lines 82 - 91, Remove the redundant `.trim() ?? ''` from `normalizeForLooseCompare`; use the string returned by `normalizeForExactCompare(value)` directly before lowercasing and removing diacritics.
🤖 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.
Inline comments:
In `@frontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.ts`:
- Around line 127-133: Update the docstring for classifyDuplicateCheckResults to
remove the unsupported claim that similar words are ordered closest-in-length
first. Document only the ordering guarantees implemented by the kindRank sort
and preserved candidate search-relevance order.
---
Nitpick comments:
In `@frontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.ts`:
- Around line 82-91: Remove the redundant `.trim() ?? ''` from
`normalizeForLooseCompare`; use the string returned by
`normalizeForExactCompare(value)` directly before lowercasing and removing
diacritics.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7b7390db-8a03-4492-a355-b942ad36a7c3
📒 Files selected for processing (18)
frontend/viewer/src/lib/activity/ActivityItemChangePreview.sveltefrontend/viewer/src/lib/components/dictionary/DictionaryEntry.sveltefrontend/viewer/src/lib/entry-editor/EntryOrSenseItemList.sveltefrontend/viewer/src/lib/entry-editor/NewEntryDialog.sveltefrontend/viewer/src/lib/entry-editor/duplicate-check/DuplicateCheck.sveltefrontend/viewer/src/lib/entry-editor/duplicate-check/DuplicateCheckSection.sveltefrontend/viewer/src/lib/entry-editor/duplicate-check/DuplicateSummaryPill.sveltefrontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.test.tsfrontend/viewer/src/lib/entry-editor/duplicate-check/duplicate-check.tsfrontend/viewer/src/locales/en.pofrontend/viewer/src/locales/es.pofrontend/viewer/src/locales/fr.pofrontend/viewer/src/locales/id.pofrontend/viewer/src/locales/ko.pofrontend/viewer/src/locales/ms.pofrontend/viewer/src/locales/sw.pofrontend/viewer/src/locales/vi.pofrontend/viewer/tests/ui/new-entry-duplicates.test.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
Probably an edge case so feel free to ignore it, but I did find this a little janky Video.mp4 |
|
I couldn't help myself Video.mp4 |
hahn-kev
left a comment
There was a problem hiding this comment.
I left some feedback, I'm happy to merge it, but there's a few things that could be improved.
I also made some minor tweaks myself. Mostly around the pill, I added a threshold for triggering the intersection check, this means that when only 50% of the check area is showing, then the pill will show or hide, previously it would only trigger on 100%, however it is a bit weird now if the duplicate check is open, then the pill shows up too early possibly. I'll leave it and you can check it out and revert it if you don't like it.
- Type DuplicateCheckWritingSystems as a Pick of the real WritingSystemService
- Move duplicateResultContainerClass into DuplicateCheck.svelte's module script
- Drop auto-expand on exact match (form jumped while typing) and slide the box in/out
- Remove always-true {#if badge}; align lite copy to "No similar words found"
- Test isSimilarWord directly with trivial inputs; clarify suffix/lexeme-attribution
test names; realistic gloss-overlap data; cross-field test via getDuplicateCheckQueries
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWYjmnKqQz7EURdNYU2YvA
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWYjmnKqQz7EURdNYU2YvA
I changed it from 50% to 16px, because 50% is great if the list is collapsed, but is too much if the list is expanded. |
Resolves #1752.
Searches existing entries as you type in the new-entry dialog and surfaces likely duplicates before you save.
Screenshots
A sticky pill is used to immediately communicate that the currently entered word or sense is a potential duplicate.
Similar words "pill"
Same form/headword "pill"
Similar words list
Collapsed:

Three kinds of matches: Same word, similar word and similar meaning: (Should there be "Same meaning"? Meh)
Note "Same word" ("Same lexeme form" in classic view), because it's a lexeme form exact match even though it's not a headword exact match:

First 3 are shown. More can be added to the DOM on demand:
Diacritic mismatch remotes an exact match to a similar match:

With the added diacritic it's an exact match:

Morph-tokens are NOT stripped before querying for potential duplicates. That might be an improvement (feels very similar to stripping diacritics, which seems like an obviously good idea).