ADFA-6597: Find a tooltip through Tooltips' (categoryId, tag) index

[davidschachterADFA]

ADFA-6597

Showing a tooltip no longer scans the whole Tooltips table.

Cause

QUERY_TOOLTIP joined Tooltips and TooltipCategories. documentation.db has no ANALYZE statistics, so SQLite's planner chose to scan all of Tooltips (46,128 rows, 13 MB) and look up each row's category. EXPLAIN QUERY PLAN showed SCAN T.

Change

The query looks the category's id up first:

SELECT T.rowid, T.id, T.summary, T.detail
FROM Tooltips AS T
WHERE T.categoryId IN (SELECT id FROM TooltipCategories WHERE category = ?)
AND T.tag = ?

• Plan: SEARCH T USING INDEX sqlite_autoindex_Tooltips_1 (categoryId=? AND tag=?), using the table's existing UNIQUE (categoryId, tag) index.
• Speed: on a desktop against the real database, a lookup goes from 6.2 ms to 0.004 ms and returns the same rows.
• Why IN, not =: if two categories ever shared a name, both rows would still come back, so getTooltip keeps reporting the duplicate as before.
• The arguments are now (category, tag), to match the new order. The query and its binding live in queryTooltip, in a new TooltipQuery.kt; getTooltip calls it with named arguments, and the test calls it too, so the test covers the order the app binds.
• Why a new file: ktlint leaves ToolTipManager.kt unformatted while its only top-level declaration, object TooltipManager, doesn't match the file name. Top-level code added there makes Spotless reformat the whole file, about 380 lines.
• QUERY_TOOLTIP and queryTooltip are internal, and QUERY_TOOLTIP is @VisibleForTesting, since only the plan test reads it. In bytecode both are public static members of a new TooltipQueryKt class (on stage the field was private in TooltipManager), so this PR adds that class to the plugin-api fat jar, which takes every :idetooltips class.

Not in this PR: the buttons query still scans TooltipButtons (54,799 rows, 5.4 MB), because that table has no index on tooltipId. The index has to be built into the database, which appdevforall/OfflineDocumentationTools#39 does. Measured: 2.4 ms -> 0.004 ms, for 696,320 bytes installed and 220,515 bytes of .br. Each PR helps on its own.

Testing

• New TooltipQueryTest (Robolectric, documentation.db's own Tooltips schema), three cases: the right row for a category and tag; two same-named categories still return both rows; the query plan uses the (categoryId, tag) index and doesn't scan Tooltips.
• Mutation checks: the old join fails the plan test ([SCAN T, SEARCH TC USING INTEGER PRIMARY KEY (rowid=?)]), = instead of IN fails the duplicate test, and swapping the arguments in queryTooltip fails both row tests (0 rows found).
• :idetooltips:testV8DebugUnitTest passes: 7 tests.
• idetooltips gains testImplementation(libs.tests.robolectric), the catalog entry other modules use.
• docs/documentation-database.md is updated.
• No screen changed, so there was no font-scale check. Not yet timed on a phone.

🤖 Generated with Claude Code

https://claude.ai/code/session_018CMZQuXT3GT7nEct2w4tg3

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 56dd696a-ee45-44f6-80c7-694b08dd5d6f

📥 Commits

Reviewing files that changed from the base of the PR and between 1e1d6c3 and e5f44a8.

📒 Files selected for processing: 4

• docs/documentation-database.md
• idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt
• idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipQuery.kt
• idetooltips/src/test/java/com/itsaky/androidide/idetooltips/TooltipQueryTest.kt

🚧 Files skipped from review as they are similar to previous changes: 1

• docs/documentation-database.md

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Summary

• Changed ToolTipManager.QUERY_TOOLTIP to find category IDs first, then filter Tooltips by categoryId and tag. This enables use of the existing unique (categoryId, tag) index.
• Used IN so matching tooltips remain available when category names are duplicated.
• Added Robolectric tests for matching and missing results, duplicate category names, and the query plan. The author reports that :idetooltips:testV8DebugUnitTest passes 7 tests.
• Updated the database documentation and added Robolectric as a test dependency.
• Reported performance: Desktop lookup time against the real database decreased from 6.2 ms to 0.004 ms. The change was not timed on a phone.
• Remaining risk: The TooltipButtons lookup still scans that table because it has no index on tooltipId. Addressing that scan requires a separate database-index change.

The tooltip lookup resolves category IDs first, then matches tooltips by category ID and tag. getTooltip uses the extracted query function. Robolectric tests check query results and index use.

Changes: Tooltip query

Layer / File(s)	Summary
Update tooltip lookup query idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipQuery.kt, idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt, docs/documentation-database.md	The query filters tooltips by category IDs and tag. getTooltip calls the query function. The documentation describes the indexed lookup and notes that the Tier 3 link lookup still scans TooltipButtons.
Test tooltip query results and index use idetooltips/build.gradle.kts, idetooltips/src/test/java/com/itsaky/androidide/idetooltips/TooltipQueryTest.kt	Robolectric tests check matching and missing results, duplicate category names, and use of the (categoryId, tag) auto-index without scanning Tooltips.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: hal-eisen-adfa, itsaky-adfa

Merge Risk: ⚪ Minimal · up to e5f44

Tooltip lookup now uses the existing categoryId/tag index while preserving matching results. No new user-facing failure or data risk is established, so no change-specific merge blocker remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: using the Tooltips (categoryId, tag) index for tooltip lookup.
Description check		The description is directly related to the changeset and explains the query change, performance goal, tests, scope, and implementation details.

Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the tags in rows
And finds the path where each one goes
The category comes first in view
The indexed match returns the clue
Then hops away beneath the moon}

_{Comment @coderabbitai help to get the list of available commands.}

[davidschachterADFA]

Independent review

An independent Claude Code review covered this PR. It has no critical, high or medium findings.

ID	Sev	Finding	Status
R2160-01	low	Nothing tested the argument order at the call site: swapping it back to (tag, category) passed all 7 tests, yet no tooltip would ever be found	Fixed in e5f44a8: getTooltip and the test both call queryTooltip, and swapping its arguments fails 2 tests
R2160-02	nit	internal const compiles to a public static final field, which the plugin-api fat jar exposes to Java plugins	Partly fixed in e5f44a8: marked @VisibleForTesting. Still public in bytecode, because the plan test needs the SQL
R2160-03	nit	@ConscryptMode(OFF) was unexplained, and this module doesn't need it	Fixed in e5f44a8: removed

Checked and found correct

• Same rows: the old and new queries agree on all 46,128 (category, tag) pairs in the real database, and on 604 edge variants.
• Older SQLite: the plan test holds on SQLite 3.8.7 and 3.32.2.
• Cause: the database has no ANALYZE statistics.

Outside this PR

• Locally, Robolectric runs these tests on the nix JDK's SQLite 3.51.2, not Android's. The 3.8.7 and 3.32.2 checks cover older versions.
• The CI job that runs unit tests ignores their failures, a project-wide setting.
• ktlint leaves ToolTipManager.kt unformatted while its only top-level declaration, object TooltipManager, doesn't match the file name, and build.gradle.kts suppresses the standard:filename lint. Renaming the object to match, or adding a second top-level declaration, makes Spotless reformat about 380 lines. That is why the helper is in a file of its own. Other files with the same mismatch were not checked.

🤖 Generated with Claude Code

[davidschachterADFA]

Independent review of the fix commits

An independent Claude Code review covered e5f44a8, which answers the earlier independent review (R2160-01 to R2160-03). It has 1 nit. The fix does what it claims: the binding order is now pinned by tests, and every mutation check in the PR body reproduces. The nit is about how the PR body describes the bytecode visibility. All are open.

ID	Sev	File:line	Finding
R2160-04	nit	TooltipQuery.kt:12, :21	PR body says the SQL field is public "as it was"; on stage it was private

R2160-04.

• Reproduced: on stage, ToolTipManager.kt:56 is private const val QUERY_TOOLTIP, a private static field. At e5f44a8, javap -p on TooltipQueryKt.class shows public final class TooltipQueryKt with public static final String QUERY_TOOLTIP and public static final Cursor queryTooltip(SQLiteDatabase, String, String). :app:assemblePluginApiFatJar copies every idetooltips class into the plugin-api jar, and generatePluginApiKeepRules writes a -keep class ... { *; } rule for each one, so R8 keeps both in release builds.
• Cause: the PR body says: "It is still a public static field in bytecode, as it was inside TooltipManager, so it is still in the plugin-api fat jar." That is true only of the PR's first commit. Compared with stage, the base the PR is reviewed against, this PR adds a new class to the plugin ABI. The queryTooltip method is new in e5f44a8 and the body does not mention it. R2160-02 covered the field; it did not cover the method or this wording.
• Suggested fix: reword the body, for example: "This PR adds TooltipQueryKt to the plugin-api fat jar: QUERY_TOOLTIP (private on stage) and queryTooltip are public static in bytecode, though internal to Kotlin callers." If the method should stay out of the Java ABI, @JvmSynthetic on queryTooltip hides it from javac. It does not remove the class from the jar.

Earlier findings

ID	Verdict	Evidence
R2160-01	Fixed	arrayOf(tag, category) in queryTooltip fails 2 of 3 TooltipQueryTest cases (expected 1 and 2 rows, got 0). The getTooltip call is not covered: queryTooltip(database, tag, category) (positional) compiles and passes all 7 tests. Named arguments guard it
R2160-02	Partly fixed, as the author says	@VisibleForTesting is present. The field is still public static (see R2160-04)
R2160-03	Fixed	@ConscryptMode is gone. The module has no other use of it, and all 7 tests pass without it

Checked and found correct

• :idetooltips:testV8DebugUnitTest at e5f44a8: 7 tests, 0 failures (GradleTaskTooltipTagTest 4, TooltipQueryTest 3), as the PR body says.
• All three mutation checks in the PR body reproduce with the failures it names. The old join fails the plan test with [SCAN T, SEARCH TC USING INTEGER PRIMARY KEY (rowid=?)]. = in place of IN fails the duplicate test (expected 2, got 1). Swapping the binding fails both row tests (0 rows).
• spotlessCheck passes at e5f44a8.
• The docs/documentation-database.md entry matches the code. No other doc names QUERY_TOOLTIP.
• The comment on queryTooltip is accurate: TooltipManager's initializer reads Environment.DOC_DB and external storage, so a test can't touch the object.
• The only other Tooltips/TooltipCategories join, in PluginDocumentationManager.removePluginDocumentationInternal, runs when a plugin is removed, not each time a tooltip is shown. Leaving it alone is reasonable.

Not verified:

• The SQL plan on a device's SQLite. This review ran it only on Robolectric's SQLite.

🤖 Generated with Claude Code

[davidschachterADFA]

Response to the review of the fix commits

ID	Status	What changed
R2160-04	Fixed in the description	It now says both QUERY_TOOLTIP and queryTooltip are public static members of a new TooltipQueryKt in bytecode, private on stage, and that the class goes into the plugin-api fat jar

The call to queryTooltip from getTooltip stays guarded by its named arguments, not by a test: TooltipManager's initializer reads Environment.DOC_DB and external storage, so a unit test can't reach getTooltip. No code changed.

🤖 Generated with Claude Code