Context
versions.models.Review has exactly one writer - the boost.org results importer - and nothing else
in the codebase creates or updates those rows. So the Reviewer achievement and that importer are one
piece of work: the badge counts rows only the importer produces, and the natural moment to update the
grants is the moment the reviews change.
Two terms from the earlier engine ticket appear below. A backfill grants what a source supports and
the site is missing, and never removes anything. A reconcile is the two-way version, which also
removes grants a source no longer supports; it stays a deliberate human action and nothing here triggers
one.
The importer is broken. It scrapes https://www.boost.org/community/review_schedule.html, which
no longer exists, and matches tables by summary="Review Results", an attribute the replacement page
does not have. It therefore finds nothing and returns quietly.
The results now come from a regularly published Antora document,
boostorg/website-v2-docs/formal-reviews/modules/ROOT/pages/review-results.adoc, served at
https://www.boost.org/doc/formal-reviews/review-results.html. Three things about that page
break the old approach beyond the URL:
- The content is HTML-escaped inside an
<iframe srcdoc="..."> wrapper.
- The section opens with a year-navigation table, and the old code takes the first table after
the heading. Pointing the existing command at the new URL would import roughly two junk rows
instead of the ~286 real reviews - and report success.
- boost.org serves the page with no charset, so
requests falls back to ISO-8859-1 per the HTTP
spec and every accented reviewer name is stored mojibaked.
Separately, the command has always created near-duplicate rows on every run, because its
update_or_create key is too narrow for the data.
Scope
The importer
- Point it at the current URL and extract the real document from the content iframe.
- Force UTF-8 decoding.
- Parse every result table in the section, not the first. Match on the header row and walk until
the next h2, so the navigation table is rejected and each year's table is picked up.
- Replace the narrow
update_or_create key with a normalized fingerprint - submission, submitter
and review dates, accent-stripped, lowercased, punctuation removed - so a re-import updates in
place, pre-existing duplicates collapse, and a library reviewed twice on different dates stays two
records.
- Import oldest-first so the newest review gets the highest id, matching the ordering the admin and
the public page use.
- Handle superseded results via the current markup.
- Fail loudly. An unparseable page must raise, not return quietly - this also runs from a task
where nobody is watching the output.
- Time out the fetch, so a boost.org that accepts the connection and never answers cannot hold a
worker for the life of the process.
- Move the command's existing
--clean flag inside the import transaction and after the parse, so a
failed scrape cannot leave the reviews deleted.
- Discard the achievements sourced from a review before deleting it as a duplicate. A grant points at
its source row by table and id, with no database-level link back, so nothing stops the row being
deleted underneath it and a bare delete leaves a grant still counting toward a threshold.
- Only store GitHub URLs as
github_link, matched on hostname. Omit the key otherwise, so a
re-import cannot erase a link entered by hand.
- Make the review changelist usable for this data: id, review-manager and scraped-review-manager
columns, newest-first ordering, wider search, and whatever prefetching the results column needs
to avoid a query per row.
The button
- A Celery task wrapper plus an Import Reviews from boost.org button on the review changelist,
built on the existing task-button infrastructure. Gate it on delete permission rather than
change - the command can merge duplicate reviews and delete the losing rows along with the
achievements sourced from them - with a confirmation page that says what may be deleted in the
same words as the button's help text.
The achievement
- A source iterator - the function the ingestion engine walks for one achievement - yielding
(member, review) for every submitter that has a linked user.
Submitters are CommitAuthor rows and only some are claimed by a member; an unlinked one is
skipped rather than guessed at.
- Register it against its achievement, which also makes it selectable wherever a single source can be
named - the commands and the admin buttons alike.
- Bring the grants into step at the end of the import task, for this source only: sweeping every
source would walk the commit and library tables the import never touches. The additive sweep is the
right one here, since removals are already handled by item 10.
- Tests: unlinked submitters are skipped; grants and badges produced end to end; the import task's call
sequence; and one engine test that only becomes meaningful once a second source is wired - the admin's
two-way sync must refuse to offer its Apply button when an achievement is missing from the seeded
definitions.
Out of scope
- Importing upcoming / scheduled reviews. The page still lists them; nothing consumes them.
- Any change to
versions.models.Review - the fields all exist already.
documentation_link. The submission cell also carries project pages and announcement posts, and
guessing which is documentation is not this command's job.
- Reconciling from the import task. Removals go through the achievement-discard path; a reassigned
submitter leaves a stale grant until someone reconciles, which is the engine's deliberate split.
- Improving how submitters are linked to members. That is upstream of this ticket.
Acceptance criteria
Risks & considerations
Matching tables on an exact header row means a cosmetic upstream rename makes the command find zero
tables and raise. That is the trade worth taking: failing loudly beats importing nothing and
reporting success. Same for aborting on a malformed row - one bad year costs the whole import rather
than silently costing one table.
The fingerprint is deliberately flexible, so two genuinely distinct reviews of the same library on
the same dates by the same submitter would collapse. There are none today, and the dates component is
what keeps re-reviews apart.
Context
versions.models.Reviewhas exactly one writer - the boost.org results importer - and nothing elsein the codebase creates or updates those rows. So the Reviewer achievement and that importer are one
piece of work: the badge counts rows only the importer produces, and the natural moment to update the
grants is the moment the reviews change.
Two terms from the earlier engine ticket appear below. A backfill grants what a source supports and
the site is missing, and never removes anything. A reconcile is the two-way version, which also
removes grants a source no longer supports; it stays a deliberate human action and nothing here triggers
one.
The importer is broken. It scrapes
https://www.boost.org/community/review_schedule.html, whichno longer exists, and matches tables by
summary="Review Results", an attribute the replacement pagedoes not have. It therefore finds nothing and returns quietly.
The results now come from a regularly published Antora document,
boostorg/website-v2-docs/formal-reviews/modules/ROOT/pages/review-results.adoc, served athttps://www.boost.org/doc/formal-reviews/review-results.html. Three things about that pagebreak the old approach beyond the URL:
<iframe srcdoc="...">wrapper.the heading. Pointing the existing command at the new URL would import roughly two junk rows
instead of the ~286 real reviews - and report success.
requestsfalls back to ISO-8859-1 per the HTTPspec and every accented reviewer name is stored mojibaked.
Separately, the command has always created near-duplicate rows on every run, because its
update_or_createkey is too narrow for the data.Scope
The importer
the next
h2, so the navigation table is rejected and each year's table is picked up.update_or_createkey with a normalized fingerprint - submission, submitterand review dates, accent-stripped, lowercased, punctuation removed - so a re-import updates in
place, pre-existing duplicates collapse, and a library reviewed twice on different dates stays two
records.
the public page use.
where nobody is watching the output.
worker for the life of the process.
--cleanflag inside the import transaction and after the parse, so afailed scrape cannot leave the reviews deleted.
its source row by table and id, with no database-level link back, so nothing stops the row being
deleted underneath it and a bare delete leaves a grant still counting toward a threshold.
github_link, matched on hostname. Omit the key otherwise, so are-import cannot erase a link entered by hand.
columns, newest-first ordering, wider search, and whatever prefetching the results column needs
to avoid a query per row.
The button
built on the existing task-button infrastructure. Gate it on delete permission rather than
change - the command can merge duplicate reviews and delete the losing rows along with the
achievements sourced from them - with a confirmation page that says what may be deleted in the
same words as the button's help text.
The achievement
(member, review)for every submitter that has a linked user.Submitters are
CommitAuthorrows and only some are claimed by a member; an unlinked one isskipped rather than guessed at.
named - the commands and the admin buttons alike.
source would walk the commit and library tables the import never touches. The additive sweep is the
right one here, since removals are already handled by item 10.
sequence; and one engine test that only becomes meaningful once a second source is wired - the admin's
two-way sync must refuse to offer its Apply button when an achievement is missing from the seeded
definitions.
Out of scope
versions.models.Review- the fields all exist already.documentation_link. The submission cell also carries project pages and announcement posts, andguessing which is documentation is not this command's job.
submitter leaves a stale grant until someone reconciles, which is the engine's deliberate split.
Acceptance criteria
import_reviewspopulatesReviewandReviewResultfrom the current page(expect ~286 rows, 0 without a result)
--cleancannot leave the table empty after a failed scrapeRisks & considerations
Matching tables on an exact header row means a cosmetic upstream rename makes the command find zero
tables and raise. That is the trade worth taking: failing loudly beats importing nothing and
reporting success. Same for aborting on a malformed row - one bad year costs the whole import rather
than silently costing one table.
The fingerprint is deliberately flexible, so two genuinely distinct reviews of the same library on
the same dates by the same submitter would collapse. There are none today, and the dates component is
what keeps re-reviews apart.