feat(manage-tags): introduce ViewModel [DEV-only] by david-allison · Pull Request #20578 · ankidroid/Anki-Android

david-allison · 2026-03-25T06:01:11Z

Note

Assisted-by: Claude Opus 4.6
Almost entirely written by Claude, with hours of back-and-forth over design and refactorings
I drove the ideas, Claude wrote the code. Read it heavily

Note

Please request I break this up into commits if appropriate. 400LOC of well-commented non-test code is borderline to me.

Purpose / Description

Even though the UI of the tags dialog is subject to design, the ViewModel and basic operations which we will support are unlikely to change

Based on tags.tree()
Delete/clear unused/rename are supported
Collapsed/expanded state is synced with the collection
A TagName abstraction acts as a guardrail against logging tag names
Introduces a Compose-style 'Channel' for UI state notifications

Fixes

Part of issue Delete tags #10397

Approach

Build a ViewModel
Add basic logic: delete, rename, clear unused & collapsed. Claude added multiselect & deletion
Refactored ManageTagsState to use a union of Loading/Loaded/Error
Refactored to use tag.tree()
Lots of refactoring of both the code and the tests
Lots of high-level docs
Added a TagName abstraction to handle bad logging decisions
Added in indeterminate states to fix bugs in multi-selection & delete
Removed mutlti-select after further UX issues combining it with filtering

How Has This Been Tested?

Heavily unit tested

Learning

I've been using Compose more, and I like the Channel<Event> pattern to consolidate all events in 1 flow
Indeterminate selection states in a treeview is nasty
- Happy I skipped it
We need to look into OpChanges classes at a later date and add a common marker interface, the lambda return bug has hit too many times
Performance could likely be optimized, but it's a lot of complexity for an op which takes ~50ms on my emulator.
filtering + selection just feels dangerous

Checklist

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

criticalAY

Looks cool

criticalAY · 2026-04-04T18:23:44Z

+            Timber.w("findVisibleNode called while not loaded")
+            return null
+        }
+        return loaded.visibleNodes.firstOrNull { it.fullTag == tag }.also {


findVisibleNode right now is O(n) , what if there are 20k tags? (hypothetical), can we think of doing O(1)?

Trade off is memory though

Even with 20k tags, this should be under a millisecond.

After this change, we rebuild the tree from the collection, so there's little point in this - we've got to pay somewhere to build the map

criticalAY · 2026-04-04T18:31:56Z

+        private fun hasDescendantMatching(
+            node: BackendTagTreeNode,
+            parentFullTag: String,
+            searchQuery: String,
+        ): Boolean {
+            for (child in node.childrenList) {
+                val fullTag = "$parentFullTag::${child.name}"
+                if (fullTag.contains(searchQuery, ignoreCase = true)) return true
+                if (hasDescendantMatching(child, fullTag, searchQuery)) return true
+            }
+            return false
+        }
+    }
+}


KDoc imo? And could we do a single pass instead? Compute all full paths once, mark matches,
and propagate "has matching descendant" back up to parents.

I switched to a simple linear approach:

build the flattened list once, with lowercased tag for performance (removes ignoreCase = true)

Loop through the list and check. If a match occurs, flag all ancestors to be expanded.

This one has caused me multiple issues in the past Assisted-by: Claude Opus 4.6

Remove could be the number of tags, or the number of notes https://github.com/ankitects/anki/blob/2d44d4d6bc486803f9236033ad840df203c87036/rslib/src/tags/rename.rs https://github.com/ankitects/anki/blob/2d44d4d6bc486803f9236033ad840df203c87036/rslib/src/tags/remove.rs

* Based on tags.tree() * Delete/clear unused/rename are supported * Collapsed/expanded state is synced with the collection * A 'TagName' abstraction acts as a guardrail against logging tag names * Introduces a Compose-style 'Channel' for UI state notifications Part of issue 10397 Assisted-by: Claude Opus 4.6 Almost entirely written by Claude, with hours of back-and-forth over design I drove the ideas, Claude wrote the code

handled

mikehardy

dev-only, and foundation for more work (wherein I assume it'll be heavily exercised and if there are faults, they'll be flushed out). Can't think why not to merge it

lukstbit

Had some comments here but this was merged before coming back.
Mostly nitpicks but some real questions as well.

I was considering 'multi-delete', and I've decided against it.

The backend provides the functionality, but the potential UI states when combining it with a search/filter mechanism are too complex to explain well to the user:

I recommend to reconsider this. I can totally see some users using prefixes in tag names to group them and wanting to delete those in one go.

lukstbit · 2026-04-20T12:12:34Z

+    data object Loading : ManageTagsState()
+
+    data class Loaded(
+        val visibleNodes: List<TagListItem>,


Why just the visible nodes?

Feels like we should just output all tags we have and make TagListItem have a boolean flag isFilteredOut to indicate its visibility. Should help with submitList as well if we used it in the UI.

We already have the full list of tags in the ViewModel and this should also simplify the tag filtering code.

Is there a reason we'd ever want to display filtered nodes?

We'd want to filter them before they reached the RecyclerView for performance so we're only comparing deltas in visible rows, so I don't see how submitList would benefit.

I suppose we could get rid of the cache, but I don't find that compelling in itself, was there other simplification I'm missing?

david-allison added the Next version Changes to be merged in the next version, to keep the current release stable. label Mar 25, 2026

github-actions Bot added the Needs Review label Mar 25, 2026

david-allison force-pushed the tags-dialog branch 4 times, most recently from a341e75 to 7ab260b Compare March 25, 2026 13:47

criticalAY previously requested changes Apr 4, 2026

View reviewed changes

criticalAY added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Apr 4, 2026

david-allison added 2 commits April 4, 2026 21:36

docs: opChanges return type issue

a8e2e6c

This one has caused me multiple issues in the past Assisted-by: Claude Opus 4.6

david-allison force-pushed the tags-dialog branch 2 times, most recently from 5a2112f to 41223b2 Compare April 5, 2026 04:16

david-allison force-pushed the tags-dialog branch from 41223b2 to 8471769 Compare April 5, 2026 04:22

david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Apr 5, 2026

david-allison requested a review from criticalAY April 5, 2026 04:22

david-allison removed the Next version Changes to be merged in the next version, to keep the current release stable. label Apr 7, 2026

david-allison changed the title ~~feat(manage-tags): introduce ViewModel~~ feat(manage-tags): introduce ViewModel [DEV-only] Apr 7, 2026

mikehardy approved these changes Apr 19, 2026

View reviewed changes

mikehardy added this pull request to the merge queue Apr 19, 2026

mikehardy added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Review labels Apr 19, 2026

Merged via the queue into ankidroid:main with commit 8ba345e Apr 19, 2026
15 checks passed

github-actions Bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Apr 19, 2026

github-actions Bot added this to the 2.24 release milestone Apr 19, 2026

lukstbit reviewed Apr 20, 2026

View reviewed changes

david-allison deleted the tags-dialog branch April 20, 2026 13:19

This was referenced Apr 20, 2026

Delete tags #10397

Open

Manage Tags: Handle review comments #20798

Merged

Uh oh!

Conversation

david-allison commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning

Checklist

Uh oh!

criticalAY left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

criticalAY Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

criticalAY Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

david-allison Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criticalAY Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

david-allison Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

mikehardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukstbit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukstbit Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-allison Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

david-allison commented Mar 25, 2026 •

edited

Loading

david-allison Apr 5, 2026 •

edited

Loading

lukstbit Apr 20, 2026 •

edited

Loading