Skip to content

Consistent handling of no token lemmas#930

Merged
killergerbah merged 2 commits intomainfrom
no-lemmas-fix
Mar 8, 2026
Merged

Consistent handling of no token lemmas#930
killergerbah merged 2 commits intomainfrom
no-lemmas-fix

Conversation

@ShanaryS
Copy link
Copy Markdown
Collaborator

@ShanaryS ShanaryS commented Mar 8, 2026

After thinking about it some more there is some inconsistencies on how no lemmas are being handled. In #929 we need to always build the exact cache in case we are in a new session where the user hasn't collected the token yet, not just for token refreshes.

The Anki database was allowing junk tokens by allowing ones without lemmas to be stored. Even if these tokens were valid it would only match with the exact strategy and not the others which could lead to confusion. So it's best to just ignore these. I don't think it's worth trying to prune the existing entries for users as these shouldn't appear in subtitles and will naturally get pruned over time if the card/note is ever modified.

@ShanaryS ShanaryS requested a review from killergerbah March 8, 2026 07:44
@ShanaryS ShanaryS self-assigned this Mar 8, 2026
@ShanaryS ShanaryS added the bug Something isn't working label Mar 8, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 8, 2026

Deploying asbplayer with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3c58f8c
Status: ✅  Deploy successful!
Preview URL: https://51c82787.asbplayer.pages.dev
Branch Preview URL: https://no-lemmas-fix.asbplayer.pages.dev

View logs

Copy link
Copy Markdown
Owner

@killergerbah killergerbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more consistent to just consider any token with no lemmas as having itself as a lemma? Or is the intention to only let these types of tokens exist in DB as local tokens picked by the user?

@ShanaryS
Copy link
Copy Markdown
Collaborator Author

ShanaryS commented Mar 8, 2026

I considered just always appending the token if no lemmas but I didn't want it for the other use cases outside of SubtitleColoring. I should have just added a flag to control it in the first place rather than trying to change the strategy processing.

Would it be more consistent to just consider any token with no lemmas as having itself as a lemma?

No we should generally not allow non dictionary entries into the database, at least now there is much reason to. So that when we parse Anki cards or user imports we can ignore these words. But we can allow it when they manually collect it on a subtitle for better UX essentially.

Or is the intention to only let these types of tokens exist in DB as local tokens picked by the user?

Yes that's the goal. Only local tokens that the user specifically picked out.

@killergerbah
Copy link
Copy Markdown
Owner

I see, the entire diff including the last commit became a lot simpler with this change

@killergerbah killergerbah merged commit ef9926d into main Mar 8, 2026
2 checks passed
@killergerbah killergerbah deleted the no-lemmas-fix branch March 8, 2026 23:05
@killergerbah killergerbah added this to the Extension v1.15.0 milestone Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants