Signal-ranked Notable Authors for subject pages#13142
Conversation
for more information, see https://pre-commit.ci
|
Thanks for the PR! The linked issue hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently 29 open non-draft PRs ahead of yours. Possible improvements for this PR
Note: this PR uses PR triage checklist (maintainers / Richy)
Note This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting. — Richy, via PAM (Open Library's Project AI Manager) |
|
Just calling out that the new Author list does incur some backend cost. The breakdown is below and if the decision is made to minimize the cost, some solutions are posted below as well. Notable Authors — Solr/DB cost & cachingWhat the new work costsExtra Solr query —
So per uncached subject page you're adding roughly one extra Solr round-trip (less than the existing one) plus one cheap batched fetch — call it a 30–50% bump in backend work for that page. Not alarming in isolation. The reason to care: subject pages are an SEO-crawled long tail — crawlers hit thousands of rarely-viewed subjects, so front-of-house edge caching has low hit rates exactly where this cost lands. MitigationSince these lists change infrequently, this is a textbook fit for A. Memoize the notable-authors computation, keyed by normalized subject path, long TTL (6–24h). (Do this one.) Collapses both the extra Solr query and the Two implementation gotchas:
B. Trim the sample. C. (Future, if needed) precompute offline. A periodic job writing notable_authors per subject into a table/Solr field. Overkill for Phase 1 — a fallback if caching ever proves insufficient. Don't derive rep-works from the main query instead of a second query — the main query is paged to 12 works and sorted for the carousel, so you'd lose author coverage. The separate bounded query is the right structure; just cache it. Recommendation: A (+ B as a freebie). That turns the steady-state added load into one memcache GET, proportional to how rarely this data changes. |
e69a846 to
a8f2d68
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
Replaces Backend (openlibrary/plugins/worksearch/subjects.py)
Frontend (SubjectAuthors.html + subject-authors.css)
Tests: 21 tests covering decoration, caching seams, and the async ranking logic (malformed-doc guards, cap-overshoot protection, cover passthrough). Adopted a few structural refinements from @lokesh's review branch |

Closes #13135
Phase 1
Replaces "Prolific Authors" (raw
author_keyfacet count) with a signal-ranked "Notable Authors" list, per Phase 1 of the subject page redesign epic.Technical
readinglog_count) scans works once; each author's first occurrence in that signal-sorted list becomes their representative work.author_keyfacet (already computed by the main query free).site.get_manycall, mirroring the existingdecorate_with_tagspattern.Files
openlibrary/plugins/worksearch/subjects.py- newget_notable_authors_async(SubjectEngine) anddecorate_with_author_photos(page handler)openlibrary/macros/SubjectAuthors.html- new macro (replacesProlificAuthors.html, which is now unused and removed)openlibrary/templates/subjects.html- swapped macro callstatic/css/components/subject-authors.css- new card layout styles, built on existing design tokensopenlibrary/plugins/worksearch/tests/test_subjects.pynew test coverage for both the ranking logic and the photo-decoration fallbackKnown tradeoff
The 100-row sample bound means a very prolific author could theoretically be undercounted if none of their works land in the sample, unlikely in
practice since the sort already surfaces their best work early, but flagging it as a documented tradeoff rather than a silent gap.
Testing
/subjects/science_fictionrenders the new "Notable Authors" box correctly, author card with photo fallback (placeholder avatar, since local Solr data has no cover), representative work link("Flatland"), and author name link, all functioning.
python -m ruff checkpytest openlibrary/plugins/worksearch/tests/test_subjects.py -v- all passeddecorate_with_author_photos: photo present, photo missing,author thing missing entirely, empty/missing notable_authors list
get_notable_authors_async: representative-work selection from a signal-sorted sample,MAX_NOTABLE_AUTHORScap, and the no-matching-works (sparse subject) caseNot yet tested / left for reviewer awareness:
Screenshot
Stakeholders
@lokesh