Display morph type tokens in UI#2205
Conversation
github-actions
bot
added
the
💻 FW Lite
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Rebasing on top of #2192 again to pull in the renames that happened over there. |
rmunn
force-pushed
the
feat/morph-types-in-ui
branch
from
602774b to
decaf67
Compare
frontend/viewer/src/project/data/writing-system-service.svelte.ts
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
I've been looking into the failing Playwright tests. Here's what I've found so far:
Test is grabbing the first entry to use as a search filter. Now that first entry has become
Chooses the first entry to delete, then expects that entry to no longer be in the list. Thing is, it's choosing We might want a precondition that counts the number of entries that match the one chosen, and checks that the result after deletion is one fewer. ... Actually, this one is failing when I run it locally in the
Updates the first entry by prefixing it with At least, that's what I think is going on. It's hard to tell, because when I look at the Playwright trace, the visuals that it captured are from when the placeholders are still visible, rather than when the text has shown up. So I can't tell from the trace which entry it's actually capturing the text from. . Basically, I have made a little progress, but much of the Playwright tests' behavior remains a mystery for now. My next step is going to be to rebase onto the latest commit in |
rmunn
changed the title
|
It might be premature to work on the Playwright tests, actually. Maybe I should wait until we have the sorting and filtering story worked out, then see what happens with the tests. I do want to know what happened to those "8 removed" snapshots from Argos, though. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e7c3b1b7-c318-4fd8-93a6-77ba41b5def7
📒 Files selected for processing (7)
backend/FwLite/FwLiteShared/Services/MiniLcmJsInvokable.csfrontend/viewer/src/lib/dotnet-types/generated-types/FwLiteShared/Services/IMiniLcmJsInvokable.tsfrontend/viewer/src/lib/dotnet-types/index.tsfrontend/viewer/src/project/data/morph-types.svelte.tsfrontend/viewer/src/project/data/writing-system-service.svelte.tsfrontend/viewer/src/project/demo/demo-entry-data.tsfrontend/viewer/src/project/demo/in-memory-demo-api.ts
| headword(entry: ReadonlyDeep<IEntry>, ws?: string): string { | ||
| if (ws) { | ||
| return headword(entry, ws) || ''; | ||
| return this.#decorated(entry, ws) || ''; | ||
| } | ||
| return firstTruthy(this.vernacularNoAudio, ws => this.#decorated(entry, ws.wsId)) || ''; | ||
| } | ||
|
|
||
| return firstTruthy(this.vernacularNoAudio, ws => headword(entry, ws.wsId)) || ''; | ||
| #decorated(entry: ReadonlyDeep<IEntry>, ws: string): string | undefined { | ||
| // Citation forms should not be decorated with prefix/postfix tokens, only lexeme forms get decorated | ||
| return entry.citationForm[ws] || this.#morphTypesService.decorate(entry.lexemeForm[ws], entry.morphType); | ||
| } |
There was a problem hiding this comment.
Make the decorated headword searchable too.
After Line 137, the UI can show -a/a-, but the current demo search/filter path still matches only raw lexemeForm/citationForm values. That means users can now see headwords they cannot filter for, which is already the failing -a case called out in the PR notes. Please drive filtering from the same decorated headword representation.
There was a problem hiding this comment.
I fixed sorting in the demo API to not consider/include/respect morph-tokens.
TODO: It's true that the filter code should check against headword OR lexeme.
There was a problem hiding this comment.
🧩 Analysis chain
Let me look at the filter code to provide concrete guidance on where and how to fix it.
🏁 Script executed:
#!/bin/bash
# Find filter/search related code in the frontend viewer
rg -n "lexemeForm\|citationForm\|headword" --type=ts --type=svelte -C3 frontend/viewer/src \
-g '!*.d.ts' \
| grep -i "filter\|search\|match\|find" -A3 -B3 | head -100
# Also search for the filter function directly
rg -nP "filter|search" --type=ts -l frontend/viewer/src/project
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Find filter/search logic - look for where entries are filtered by text input
rg -n "filter\|search\|match" frontend/viewer/src/project/demo/in-memory-demo-api.ts -C5
# Look for entry filtering/searching code across all files
rg -rn "lexemeForm\|citationForm\|headword" frontend/viewer/src/project/ --include="*.ts" -C3 | grep -i "filter\|search\|match\|includes\|toLowerCase" -A3 -B3 | head -120
# Also search for entry list / filter entry functions
rg -rn "filterEntries\|searchEntries\|getEntries\|filter.*entry\|entry.*filter" frontend/viewer/src/ --include="*.ts" -C5 | head -120
Length of output: 883
🏁 Script executed:
#!/bin/bash
# Find filter/search logic in the demo API
rg -n "filter|search|match" frontend/viewer/src/project/demo/in-memory-demo-api.ts -C5
# Find getEntries or similar entry-listing methods with search/filter params
rg -rn "getEntries" frontend/viewer/src/project/ -C5
# Find where entries are filtered by text (lexemeForm, citationForm, headword)
rg -rn "lexemeForm|citationForm" frontend/viewer/src/project/demo/in-memory-demo-api.ts -C5
Length of output: 4097
@myieye, thanks for the sorting fix and the TODO note. The filterEntries standalone function in in-memory-demo-api.ts is the concrete place to fix — it already checks raw lexemeForm / citationForm, but needs to also include the decorated values. Since InMemoryDemoApi already holds #morphTypesService, the change is small:
// Pass morphTypesService into filterEntries
function filterEntries(entries: IEntry[], query: string, morphTypesService: MorphTypesService): IEntry[] {
return entries.filter(entry => {
const decoratedHeadwords = Object.keys(entry.lexemeForm ?? {}).map(ws =>
entry.citationForm[ws] // citation form is never decorated (matches `#decorated` logic)
? undefined
: morphTypesService.decorate(entry.lexemeForm[ws], entry.morphType)
);
return [
...Object.values(entry.lexemeForm ?? {}),
...Object.values(entry.citationForm ?? {}),
...decoratedHeadwords,
...entry.senses.flatMap(sense => [...Object.values(sense.gloss ?? {})]),
].some(value => value?.toLowerCase().includes(query.toLowerCase()));
});
}Then at the call site in getFilteredEntries:
if (query) entries = filterEntries(entries, query, this.#morphTypesService);The citationForm guard mirrors the same logic already in WritingSystemService.#decorated, so the two stay in sync — if the entry has a citation form for a given WS, the filter won't add a phantom decorated hit for that WS. The exemplar filter is a separate concern (index-tab navigation), but the same pattern would apply there if you want consistency.
🧠 Learnings used
Learnt from: CR
Repo: sillsdev/languageforge-lexbox PR: 0
File: frontend/viewer/AGENTS.md:0-0
Timestamp: 2026-02-04T14:30:07.371Z
Learning: Applies to frontend/viewer/**/*.{ts,tsx,svelte} : Use TypeScript types and API interfaces generated from .NET via Reinforced.Typings - must rebuild backend (dotnet build backend/FwLite/FwLiteShared/FwLiteShared.csproj) when .NET models or JSInvokable APIs change
myieye
force-pushed
the
feat/morph-types-in-ui
branch
from
8ff898e to
312b763
Compare
Morph types now show leading/trailing tokens in headword, but do not yet have a dropdown for editing them in the entry UI.
Lexeme forms should be decorated with prefix/postfix tokens according to the morph type, but citation forms are meant as "overrides" and should be reproduced exactly as-is, without morph type tokens. This is the rule used by FLEx for how it displays words, so FW Lite should do the same. As a bonus, there is now only one `headword` function in the writing system service, instead of two functions with the same name that did two slightly different things.
We don't need all the morph types for testing, just a few of them.
Now that morph types show prefix and postfix tokens, "-a" was sorting before "-UPDATED-" so the updated entry was no longer the first one in the list.
rmunn
force-pushed
the
feat/morph-types-in-ui
branch
from
312b763 to
6eab38b
Compare
|
Argos interface won't let me approve the visual changes because "You must be a reviewer to approve or reject changes". But I've double-checked them, and all of them are expected: "aji" becomes "-aji" because leading tokens are visible now, and so on. Since everything else passes and the PR is approved, I'm going to merge it. |
2 participants
Resolves #2223.
(Originally part of #1848, but not anymore.)
Currently merging into #2192, which has become our main branch for the Morph Types feature and will stay open until all parts of the feature are ready (thus allowing us to release other features while the Morph Types branch isn't ready yet).
Morph types now show in FW Lite UI, but do not yet have a dropdown to choose a different one. (Update: Decided in team meetings NOT to have a dropdown for changing morph types, at least not yet, because of all the complexity involved in switching from a MoAffixMsa to a MoStemMsa or vice versa in liblcm. There are corner cases that need tests and UI warnings like FieldWorks has, and the decision was made to postpone that feature. If someone needs to switch the morph type of an entry, they're probably a linguist who already has FW Classic, so they can do it in FW Classic and then sync).