Repository navigation
Librarian Tasks page - #13710
Librarian Tasks page#13710
Conversation
A guided first-contribution flow for newcomers: orientation, three practice books with instant verdicts, a list of books that need a hand, a one-field wizard with evidence from outside catalogs, sibling spelling counts and link-outs, a receipt, and fixture-backed status and review pages. Phase 1 is a click-through proof of concept. Book records, sibling editions and the reading log are live; outside evidence, practice and status states are fixtures; nothing is saved. Pages are web.py handlers rendering Jinja inside the site shell; the engine is openlibrary/first_edits/.
Cuts the parts of the walkthrough that were not pulling their weight: practice mode, the "Your suggestions" status page, the librarian review preview, the list tabs and the "only sure things" filter, confirm mode and conflict answers. plan.md records each one under "Deferred from phase 1" with the commit to recover it from. Also restyles the list rows: a bottom border instead of a card, since only the task link inside is clickable, and one font size across the field label and the evidence text.
The list is an admin-ish surface, so the rows were carrying too much air: halve the row inset, drop the vertical padding on each task link, and remove the "First edits" overline above the page title. Also reword the readers line to describe shelves rather than reads, and update the docs for the production-key demo set.
The demo set was eight editions seeded by ISBN into a local database, which meant a server running production data had nothing to show. Each entry now carries its production edition key alongside the ISBN, and the set covers 53 popular editions with real gaps, each with a matching evidence fixture. Readers counts come from each work's production reading-log count.
# Conflicts: # openlibrary/i18n/messages.pot
Answers Lisa's ask for non-IA identifiers. Both fields are enabled in scope.json at "strong" evidence, fill mode only. Identifiers don't fit the evidence model the other fields use: the number on a Library of Congress record is of course its own LCCN, so quoting it back proves nothing. What a newcomer can judge is whether that record describes the book on this page, so the task page is that comparison — the candidate record's year, publisher, page count and title against ours — plus three checks (format, collision with a sibling edition, corroboration) and a list of the traps nobody can check for them. An answer that writes an identifier is gated behind an explicit "I opened the record" tick, since a wrong identifier corrupts record matching silently. Saying "different edition" or "not sure" stays ungated. - openlibrary/utils/oclc.py: OCLC normalizer, the Python counterpart of parseOclc/isValidOclc, alongside the existing utils/lccn.py. - first_edits/identifiers.py: specs, format/collision/match checks. - contribute.js validates in the browser with the same rules. - Sibling display changes from spelling-count chips to one line per other edition; source blurbs drop out with the popovers that held them. - docs/first-edits/identifiers.md is the write-up for librarians.
# Conflicts: # openlibrary/i18n/messages.pot
# Conflicts: # openlibrary/plugins/openlibrary/code.py
The list page becomes three tabs, each a URL: the orientation at /contribute, the reader's own shelves at /contribute/yours, and the former list at /contribute/needed. /contribute/start redirects. Task rows show what a fix is worth (points in scope.json, mirroring the Edition Scorecard) and a task page remembers which tab it came from. This splits the project into its three parts: supply (which books are candidates, first_edits/supply.py), ranking (which to show first, first_edits/ranking.py) and the task page. A tab is a supply plus an ordering, so the tabs deferred for now are each a new candidates function and no new page. Your books reads the reading-log table directly rather than the Solr-backed log, which pages at 100 and carries no ISBNs: one query, one batch fetch of the logged editions, filtered by the evidence set, with a capped scan for works logged without an edition.
"What you can't break: nothing" over-promised; a careless answer does make work for a librarian. The second card now says what happens to a suggestion and the third says that "I'm not sure" is the right answer when in doubt. The cards are a grid with equal heights, so their auto rows were stretching and the titles sat at different heights across the row; pin content to the top. Adds the circle-help glyph.
scripts/first_edits/build_fixtures.py finds popular editions on openlibrary.org whose records are missing a field or hold a value worth checking, asks outside catalogs what they say, keeps the editions where that evidence makes a task under the current scope, writes the fixtures, and can import the same editions (plus a few siblings) into the local site. Sources are HathiTrust's Bib API (MARC-XML by ISBN, with OCLC and LCCN fallbacks) and the Internet Archive scan's own metadata for editions with an ocaid, both keyless. Google Books joins when a key is set; the LoC SRU host the plan named no longer answers. The scan is an exact match, so "ocaid" counts as one in the evidence builder. This run added 50 editions (103 in the demo set): 27 missing a language, 23 with a publisher that differs from the catalog, 10 missing a page count, 6 missing an identifier at strong evidence. 48 imported locally; two were rejected by the importer (one independently published, one tripping a matcher bug on a local record).
Same look as .ds__tabs: a scrolling row flush with the page edge, the current tab bold with a dark bar, a lighter bar on hover. A hidden bold twin sizes each tab for its heaviest state so going bold reflows nothing. Drops the "First edits" overline on the lists and shortens the Your books lede.
/contribute is one dashboard: a rail of missing-field filters (language, publisher, identifiers) over the most-read works with an edition missing that field, straight from Solr's edition child docs. A task is an empty field; the task page links out to WorldCat, Google Books and the Library of Congress with per-field directions and fetches nothing from them. Drops the fixture evidence, demo set, Your books, initiatives and sample stats, along with evidence.py, compare.py, sources.py and build_fixtures.py.
for more information, see https://pre-commit.ci
Answers now go through the normal edition save, like the edit form: a new revision with the librarian's comment, kind edit-book, and data.source "contribute" so the dashboard can find them. Identifiers are added to the record's existing list, never replacing it. Bad answers re-render the task page with the error and the typed value kept; "I couldn't find it" saves nothing. Every /contribute page is now for librarians and up; visitors go to login. The dashboard's right column lists the librarian's recent fixes, and the review-queue wording and review_wait_days setting are gone, since nothing is reviewed before it goes live.
The task page now shows the edition's record with the asked-about field in place, tallies how other editions fill it, and offers one-click suggestions where siblings are good evidence (language). The language answer uses a searchable ol-options-popover, which grows a filter input for long lists; ol-popover re-anchors without flipping when its content resizes.
The feature started as a first-edit walkthrough and is now a librarian dashboard at /contribute, so the old name no longer describes it. The package moves to openlibrary.contribute, the CSS namespace from fe- to contrib-, and comments follow.
lokesh
changed the title
The scope file promised configurability the code couldn't deliver, so the field points now live in one dict in tasks.py. Smaller modules fold into tasks and save, the client-side pick card gives way to /contribute/one, and only skips are remembered in the session, so a rejected answer no longer hides its task. Unused CSS, icons and stale comments go with them.
The counts and lists were four uncached block-join queries per page load, each capped by timeAllowed; on production data they timed out and Solr returned partial results (Identifiers 0, an empty Most needed list). - Drop the per-filter counts from the rail. - Cache each filter's top 200 picks in memcache for an hour, refilled in the background, with no timeAllowed so the fill completes. - Never cache partial results; Solr responses now expose partialResults.
lokesh
added
Needs: Review
openlibrary-bot
left a comment
openlibrary-bot
left a comment
There was a problem hiding this comment.
Rename /contribute → /tasks (minimum change). The URL is only wired here in contribute.py; the Python module/package names, the CSS filenames, and the template/JS comments can stay as they are (cosmetic). The templates build their links from the URLs this handler passes in, and there's no nav link to update, so these 11 lines are the whole change. You can add them all to a batch and commit together.
Suggested by Ada on behalf of Mek.
The librarian dashboard now lives at /tasks. Only the URL strings in contribute.py change; the Python module/package, templates and CSS keep their existing names.
The task and receipt pages move from /tasks/task/OL…M/<field>[/done] to /tasks/OL…M/<field>[/done]. The OLID pattern can't collide with /tasks/one.
lokesh
changed the title
There was a problem hiding this comment.
Wooohooo niiiice! I think this is an exciting experimental UI 😊 I don't want to dote too much on code quality stuff, but I'm seeing here a problem I've been seeing in a lot of claude PRs around variable naming, which I'd love to quash!
QA:
Bug (blocker): On testing, clicking "Fill in language" takes me to the book page now, not to the task page like it did during your demo. So I can't test that flow
Code blockers:
- Use solr helper methods instead of raw solr queries
- Opaque variable naming/noun introduction! See below.
I think a lot of the names/nouns introduced by this PR are too generic and say almost nothing and require understanding basically the entire feature before you can understand what the noun/concepts actually is. E.g. "Task" or "part" or "record" or "gate" or "source" or "note". I notice generic nouns/names like these are a common bad habit from AI, which I think we need to fix in our agent docs. These nouns make the code very opaque to humans, and makes reviewing the code slow and error prone, since we shouldn't have to learn a new language for every PR.
In general, new features/PRs should:
(1) Avoid introducing too many new nouns if possible. If an existing noun or a compound noun can serve the concept, use it.
(2) Try to make function names be at least a little understandable when read out of context ; this might require making simple nouns compound nouns (eg. "LookupSource" instead of "Source"). Add just enough domain context to make it distinct if you saw it in a grep or stack trace. If the name wouldn’t give a new engineer a decent guess at its role, improve it.
(3) Functions should almost always be verbs
E.g. instead of "gate" (what is a gate? Who is gating? What is it gating for? Is it a verb or a noun, a function or a class instance?), we can go for something like "require_librarian_ permissions" or "require_contribute_access" . Immediately clear what this function is doing regardless of what context it's in now.
What do you think of these guidelines? Can you ask claude to apply it to this PR? Are the results better?
| from dataclasses import dataclass | ||
|
|
||
| # The fields on offer, most valuable first. Points are the Edition Scorecard weights. | ||
| POINTS = { |
There was a problem hiding this comment.
Can we use the EditionScorecard python file directly instead of duplicating these points?
There was a problem hiding this comment.
We looked at this closely and would like to keep POINTS local for now — leaving this thread open for your call.
The blocker is that EditionScorecard (auto-generated from edition_scorecard.yml) doesn't expose a field → weight mapping we can import. Its checks are attributes spread across section classes, each under its own name, so there's no single dict to reuse. Three of our five weights do line up with a same-named check, and we've documented the correspondence in a comment on POINTS:
languages==language(15)number_of_pages==page_count(10)publishers==publisher(2)oclc_numbers(25) has no dedicated check, but like an ISBN it satisfies the scorecard'slibrary_optionscheck (also 25)lccn(25) has no scorecard counterpart at all
So importing wouldn't actually remove the duplication — we'd still hand-maintain the lccn/oclc cases and would now be coupled to a generated structure that doesn't model what we need. Happy to revisit if you'd prefer we add a proper field → weight accessor to the scorecard itself as a follow-up; that'd be the clean way to share it.
| def _solr_picks(fields: list[str], limit: int) -> list[list]: | ||
| """``[edition key, readers]`` for the most-read works with an edition missing one of ``fields``.""" | ||
| edition_q = missing_query(tuple(fields)) | ||
| result = get_solr().select( |
There was a problem hiding this comment.
Let's not use a raw solr select query ; these are rife with little performance gotchas and require an understanding of intricate solr query syntax, both of which we try to hide behind our run_solr_query helper. Something like this should do the trick:
resp = run_solr_query(
WorkSearchScheme(solr_editions=True),
{'q': '*:*'},
rows=limit,
fields=["key", "readinglog_count", "editions"],
facet=False,
request_label="BOOK_SEARCH_READABLE_COUNT",
)AI should be able to figure out the rest.
There was a problem hiding this comment.
Agreed — a raw Solr select is the wrong long-term shape here for exactly the reasons you give. This pass deliberately scoped to the naming/structure changes you flagged (field-guide rename, verb-based Python/JS names), so we haven't reworked the query layer in these commits. Leaving this thread open so it stays on the radar — the reshape (routing it through the existing Solr helper/abstraction rather than a hand-built select) is best done by @lokesh as a focused follow-up rather than folded into this review.
There was a problem hiding this comment.
Should we rename this to tasks.py ? Since the endpoint is now /tasks . To keep it consistent and avoid introducing a new noun.
There was a problem hiding this comment.
Reasonable — keeping the module name consistent with the /tasks endpoint makes sense. We held off in this pass for two reasons and are leaving it open for you/@lokesh to decide:
- There's already an
openlibrary/contribute/tasks.py(theTaskdataclass +POINTS) in the sibling package. Renaming this plugin file totasks.pywould put twotasks.pyin closely-related trees — importable (different packages) but easy to confuse when reading. - The rename touches the route registration /
delegate.pagewiring, so it's a bit more than a file move and sits outside the greenlit scope for this review (which was the field-guide rename and the verb-based Python/JS renames).
No objection to the rename itself — just flagging the naming collision so whoever does it picks a name that stays unambiguous.
There was a problem hiding this comment.
Adding a filter to the options popover is bringing bringing this component very close to the ol-select-popover, now? Can those be consolidated?
Reverts 113298d. /tasks/OL…M matches the readable-URL edition pattern (/\w+/OL\d+M), so ReadableUrlProcessor redirected every task to /books/OL…M.
Per review (cdrini): avoid opaque generic nouns/verbs so the code reads on its own. - _gate -> require_librarian_access (a verb that says what it enforces) - Source -> LookupSource, Note -> GuidelineNote (what the dataclass holds) - _record/"record" -> _edition_field_rows/"record_fields" (what it is) - _book's 'parts' -> 'edition_line_parts' Kept 'Task' (the domain object of a feature literally named Tasks) and the user's free-text 'note' as-is.
It was the only get_solr().select() call left UNLABELLED; add a CONTRIBUTE_TASKS request label like every other Solr caller, so the query is attributable in Solr's request logs.
lokesh
changed the title
Addresses cdrini's review on internetarchive#13710 (code readability; no behavior change): - Rename the "playbook" concept to FieldGuide (contribute/playbooks.py -> field_guides.py, get_playbooks -> get_field_guides, link_outs -> build_link_outs). "playbook" collides with the Ansible term and reads as a metaphor removed from the task. - Rename the opaque private helpers in contribute.py to verb-based public names (e.g. _gate -> require_librarian_access, _tally -> count_sibling_values, _recent -> get_recent_fixes, _rows -> build_task_rows, _rail -> build_filter_rail, _book -> build_book_header, _display -> format_field_value, _edition_field_rows -> build_edition_field_rows, the URL builders -> build_*_url). Every call site and template data-key updated in lockstep. - Keep tasks.POINTS local rather than importing from EditionScorecard, and correct the explanatory comment: the scorecard exposes no field -> weight mapping (its checks are attributes across section classes under their own names), three weights match a same-named check exactly (language 15, publisher 2, page_count 10), OCLC feeds the library_options check (25), and lccn has no scorecard counterpart.
Addresses cdrini's review on internetarchive#13710 (code readability; no behavior change): rename the opaque JS helpers to say what they act on and add jsdoc with types: readSkipped -> readSkippedTaskKeys, markSkipped -> markTaskSkipped, hideSkipped -> hideSkippedTasks, initList -> initTaskList, initSkip -> initSkipButton, initSuggestions -> initSiblingSuggestions, initIdForm -> initIdentifierForm, clearReceipt -> clearSavedReceipt.
|
@lokesh heads-up before your next push: two commits just landed on What these commits address from @cdrini's review:
Left open for you to drive:
One build note: the |
Open Library Tasks: a librarian dashboard at /tasks Surfaces the most-read works with an edition missing a field (language, publisher, LCCN/OCLC), filterable from a rail backed by Solr edition child docs. The task page shows the record with the field highlighted, tallies sibling values, offers one-click suggestions, and links out to WorldCat, Google Books and the LoC. Answers save through the normal edition revision (source “contribute”); identifiers append and are gated behind an “I opened the record” tick. “Save and next” moves to another task in the same filter. Librarian-only, no review queue; filter lists cached in memcache. Co-authored-by: Michael E. Karpeles (Mek) michael.karpeles@gmail.com
State: ready-for-review
Updated: 2026-10-10T19:05Z
Owner: ada-77 (pr-13710-librarian-tasks)
Adversarial-review: ada-77 subagent — a0f9a91 — PASS (one finding, POINTS/OCLC comment accuracy, fixed in this SHA)
Verified by RUNNING (Playwright headless, logged in as the dev
openlibrarylibrarian, aftermake css && make js)/tasksrenders 200 with book rows + task links; filter views load/tasks/task/OL…M/<field>stays on the task page and does not redirect to/books/…— the exact path that regressed beforenumber_of_pageswritten and read back via/books/OL…M.json; language picker persists/languages/engpre-commitgreen on all 7 changed files; Generate-POT consistentVerified by READING
edition_scorecard.py; thePOINTScomment matchescontribute.py/contribute.jsidentifier is verb-based in shipped source (grep of HEAD a0f9a91)Scope of these two commits (addressing @cdrini's review)
POINTSkept local), raw Solr select (catalog/onix/onix.py doesn't handle books with multiple title tags #3), rename plugin file totasks.py(Reset Password form for Admins doesn't work #5)Does NOT do
Next steps
git pullFirstEdits (heada0f9a9159) before any follow-upBlocked on
Fixing a gap in the catalog today means finding a record that needs work, opening the full edit form, and working out on your own where the answer lives. This PR adds Open Library Tasks at
/tasks, which turns that into one small task at a time: one book, one missing field, with the lookups already lined up.Librarians only, for now.
Technical
openlibrary/plugins/openlibrary/contribute.py. These are web.py pages rendering Jinja through a one-line Templetor shim, the same pattern/developers/designuses, because the site shell can't be composed from a FastAPI router yet.openlibrary/contribute/supply.py: candidates from Solr's edition child docs, most-read first. Each filter's top 200 are cached in memcache for an hour and refilled in the background; partial Solr results are never cached.tasks.py: a task is derived, never stored. Field points are the Edition Scorecard weights.field_guides.py: per-field copy and link-outs.save.py: validation and the save.data.source = "contribute"so the dashboard can list your recent fixes.ol-options-popovergains a filter input for long lists andshow-selectionfor use as a form field (the language picker).ol-popoverre-anchors when its content resizes.Risk
Low. Tasks is a new page that only librarians can reach, and nothing else on the site changes. Every answer is saved as a normal edit, so it shows in the history and can be reverted like any other. Identifier answers are checked before saving and need the librarian to confirm the record matches. The main risk is a librarian entering a wrong value faster than before. That's the same risk as the regular edit form, with more guidance on the page.
Performance
Testing
openlibraryuser works) and visit/tasks.ocm…number to see it tidied, and submit without the confirmation to see it refused.pytest openlibrary/tests/contribute.Screenshot
see above
Stakeholders
@mekarpeles @cdrini @RayBB