PR #244 [searching] is prone to false positives [ALSO: database integrity questions about safely/synchronously deleting "videos" from both/all DB's]

[deldesir]

The queries made to xklb-metadata.db return titles to be used as terms to fetch videos records from metadata.db. What if different videos have the same title!

[holta]

The challenge will likely be establishing a primary key (much like a UUID / GUID) for every video-or-similar, cleanly guaranteeing integrity across both (all 3) databases:

CLARIFS:

• Both above SQLite databases live in /library/calibre-web — RECAP:
  • Remember that metadata.db (for "book" and "video" titles, authors, descriptions and not much else!) has a quite fixed schema that we cannot change, as it's a de facto global standard dating back to Calibre's original introduction in ~2006.
  • Whereas the 2nd database xklb-metadata.db, that we added to IIAB Calibre-Web earlier this year to handle videos (especially for scraping of videos + their transcripts from the Internet, ETC), is defined by our very good friend who is the designer xklb — he might have suggestions for us?
• While likely not relevant to this design discussion, also just be aware of a 3rd SQLite database /library/calibre-web/config/app.db that is tangentially involved here. Though this 3rd DB is really only for (1) usernames/passwords (2) Calibre-Web settings, AND (3) bookshelves (that teachers use to organize books + videos!)

Related:

• PR Support searching videos [subtitles, captions, transcripts?] #152
• Search videos by subtitles/captions/transcripts [AND video title AND video description AND video author AND video publisher?] #197
• Should "Delete this Shelf" allow you to auto-delete all its "books" too? #204
• PR Query captions/subtitles [e.g. video transcripts] from xklb-metadata.db #244

cc: @mabuelhagag, @avni, @codewiz

[holta]

Related IDEA from @deldesir:

• At the same time we should very likely transition to (unify our db approaches around) SQLAlchemy — rather than Python’s sqlite3 library that we're currently using to manipulate /library/calibre-web/xklb-metadata.db — so as to better align with @OzzieIsaacs’s upstream Calibre-Web!

[avni]

For metadata.db, according to the calibredb manual, these are the available book fields:

Available fields: author_sort, authors, comments, cover, formats, identifiers, isbn, languages, last_modified, pubdate, publisher, rating, series, series_index, size, tags, template, timestamp, title, uuid

uuid may be a unique identifier for a given book? It may still be helpful to map the relevant columns across the three tables, metadata.db, xklb-metadata.db, app.db to support solving this (i.e., a mini-specification). Foreign keys can be used to map across tables if that is not already being done.

[holta]

Recap:

• PR Query captions/subtitles [e.g. video transcripts] from xklb-metadata.db #244 is on hold until this question is answered.

• PR New xb.py: Possible schema to standardize xklb db's + main operations? [need to align w/ Calibre-Web's metadata.db & app.db!] #253 is starting to get closer to answering the question.

[avni]

The queries made to xklb-metadata.db return titles to be used as terms to fetch videos records from metadata.db. What if different videos have the same title!

I'm seeking clarification on the problem statement, i.e., what does false positive mean, what's a use case that shows the problem?

If different videos have the same title, I think that's okay right? Hypothetical use case: 2 different people in different accounts may have uploaded videos with the same title coincidentally. An IIAB user downloads those two videos into their IIAB Calibre-Web instance. When they search for a term that includes the videos' title, we want both of these videos to return. Not clear to me yet what false positive means.

[holta]

what does false positive mean

The currently proposed PR #244 searches thru video transcripts yes, but it incorrectly returns every video that contains the same title. That's an obvious bug, and we should keep very focused on fixing this bug. Recall from 2-3 weeks ago (PR #244 originally from Aug 30) that poorly named variable term (should be termList or titleList ?) that awkwardly accumulates[*] both search query and video titles into the same list?

term = [term] + other_terms

That's Line 982 of cps/db.py in PR #244 here: https://github.com/iiab/calibre-web/pull/244/files#diff-204c1e4c10ba05516a4a6ed88fed4a34a133e781e4db0c80a4d96cd64f0b268aR982 (hope that helps?!)

[*] This hack of putting search string and video titles together into the very same list... is a fine proof-of-concept for now... but probably should not be merged! 😅

[avni]

1. The term variable including results from get_search_terms() (essentially search results) is definitely odd/incorrect.
2. side note: consider renaming get_search_terms() to be more reflecting of what it does - right now, it only searches captions (I left a comment on line 49 of lb_search.py for this)

3. This hack of putting search string and video titles together into the very same list... is a fine proof-of-concept for now... but probably should not be merged!

Agree

4. returns every video that contains the same title.

From what I can tell there's a chance that the search results contain the same video multiple times, which is not desired. If there are multiple videos with the same title (coincidentally) and a user is searching for that term, every unique video should be returned. You could/perhaps should also make sure the videos returned are unique (using a PK if available), which is perhaps what this is trying to address? I may be missing something though!

[avni]

returns every video that contains the same title.

After speaking with @holta further on Friday, separating out the search string and video titles into separate variables should resolve this so that false positives do not return! 🚀