From 0aa516ff5401c2404a6f8c7513ce385f6cde2989 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Mon, 14 Sep 2026 16:35:44 -0700 Subject: [PATCH 1/5] Re-apply the non-BISAC nonfiction reset a release later (PP-5129) Hold in draft. Merge only after the release containing #3726 and #3736 has gone out. The release N repair -- migration 52d1bbdd4671 plus the startup task that dispatches the re-score -- is exposed for as long as any old code is running. Anything reaching Subject.assign_to_genre before the re-score lands consumes the reset, and code on the superseded rules re-stamps checked=True with the same wrong value. helpers/migrate.yml stops the scripts container, where every Celery worker runs, before migrating, so the worker path is safe. The web containers are not: they are recycled after the migration and can reach assign_to_genre through a presentation recalculation. Fargate deployments are not governed by that playbook at all. Running the reset again a release later closes both, because by then no old code is live anywhere. The task is idempotent, so if the release N repair took, this is a no-op. Re-scoring is left to the nightly classify_unchecked_subjects: with no old code to lose the reset to, dispatching it here would buy nothing. Same shape as the null-audience repair, where startup task 2026_06_17 re-ran what 2026_05_12 had dispatched a release earlier. Co-Authored-By: Claude Opus 5 --- ...9_14_reapply_non_bisac_nonfiction_reset.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py diff --git a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py new file mode 100644 index 0000000000..80329f0a69 --- /dev/null +++ b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py @@ -0,0 +1,43 @@ +"""Re-apply the non-BISAC nonfiction reset now that no old code is running. + +Release N repaired these subjects: migration 52d1bbdd4671 reset checked=False +on BISAC subjects stored as nonfiction because an unresolvable code fell +through the ruleset catch-all, and startup task +2026_09_14_reclassify_non_bisac_nonfiction_subjects dispatched the re-score. + +That repair is exposed for as long as any old code is still running. Anything +reaching Subject.assign_to_genre before the re-score lands consumes the reset, +and code running the superseded rules re-stamps checked=True with the same +wrong value -- silently, and with nothing to revisit the subject afterwards. +The deploy stops the Celery workers before migrating, so they are safe, but the +web containers are recycled after the migration and Fargate deployments are not +governed by that playbook at all. + +This task exists to close that off. Running it a release later means no old +code is live anywhere, so the reset it applies cannot be consumed under the old +rules. It is idempotent: if the release N repair took, this finds nothing to do +and the run is a no-op. + +Re-scoring is left to the nightly classify_unchecked_subjects. There is no need +to dispatch it here, because by now there is no old code to lose the reset to. + +This is the same shape as the null-audience repair, where startup task +2026_06_17 re-ran what 2026_05_12 had dispatched a release earlier. + +TODO: Remove this task, the release N startup task, and +reset_non_bisac_nonfiction_subjects once this has run on all deployments +(PP-5129).""" + +from __future__ import annotations + +import logging + +from celery.canvas import Signature +from sqlalchemy.orm import Session + +from palace.manager.celery.tasks.work import reset_non_bisac_nonfiction_subjects +from palace.manager.service.container import Services + + +def run(services: Services, session: Session, log: logging.Logger) -> Signature | None: + return reset_non_bisac_nonfiction_subjects.s() From 2cdeec2bd938bba4a60e7bfffbf097a5e568fa69 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Mon, 14 Sep 2026 17:15:30 -0700 Subject: [PATCH 2/5] Point the re-apply task at the startup task, not the migration The release N repair is now a startup task rather than a migration, so the reference to migration 52d1bbdd4671 no longer resolves. Co-Authored-By: Claude Opus 5 --- .../2026_09_14_reapply_non_bisac_nonfiction_reset.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py index 80329f0a69..ebecaeb641 100644 --- a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py +++ b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py @@ -1,16 +1,16 @@ """Re-apply the non-BISAC nonfiction reset now that no old code is running. -Release N repaired these subjects: migration 52d1bbdd4671 reset checked=False +Release N repaired these subjects: startup task +``2026_09_14_reclassify_non_bisac_nonfiction_subjects`` reset ``checked=False`` on BISAC subjects stored as nonfiction because an unresolvable code fell -through the ruleset catch-all, and startup task -2026_09_14_reclassify_non_bisac_nonfiction_subjects dispatched the re-score. +through the ruleset catch-all, and chained the re-score behind it. That repair is exposed for as long as any old code is still running. Anything reaching Subject.assign_to_genre before the re-score lands consumes the reset, and code running the superseded rules re-stamps checked=True with the same wrong value -- silently, and with nothing to revisit the subject afterwards. -The deploy stops the Celery workers before migrating, so they are safe, but the -web containers are recycled after the migration and Fargate deployments are not +The deploy stops the Celery workers before the migrate step, so they are safe, +but the web containers are recycled after it and Fargate deployments are not governed by that playbook at all. This task exists to close that off. Running it a release later means no old From 3b42a47d58f5c1712420ae6b5923b7625276b774 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Tue, 15 Sep 2026 07:40:38 -0700 Subject: [PATCH 3/5] List the whole repair surface in the cleanup TODO Review nit: the TODO named this task, the release N startup task and reset_non_bisac_nonfiction_subjects, but not BISACClassifier.contradicts_stored_fiction. That method's only non-test caller is the Celery task, so removing the three listed items would have orphaned it. Adds it, plus the script and bin wrapper, so the repair lifts out in one pass. Co-Authored-By: Claude Opus 5 --- .../2026_09_14_reapply_non_bisac_nonfiction_reset.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py index ebecaeb641..941fba7931 100644 --- a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py +++ b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py @@ -24,9 +24,11 @@ This is the same shape as the null-audience repair, where startup task 2026_06_17 re-ran what 2026_05_12 had dispatched a release earlier. -TODO: Remove this task, the release N startup task, and -reset_non_bisac_nonfiction_subjects once this has run on all deployments -(PP-5129).""" +TODO: Remove the whole repair once this has run on all deployments (PP-5129): +this task, the release N startup task, ``reset_non_bisac_nonfiction_subjects`` +with its script and bin wrapper, and +``BISACClassifier.contradicts_stored_fiction``, whose only non-test caller is +that task.""" from __future__ import annotations From c3ee934fcbc307fe967500720e7067ac8e0c19c8 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Fri, 2 Oct 2026 11:52:33 -0700 Subject: [PATCH 4/5] Trim the docstring to what the release N task does not already say Co-Authored-By: Claude Opus 5 --- ...9_14_reapply_non_bisac_nonfiction_reset.py | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py index 941fba7931..d706ab9342 100644 --- a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py +++ b/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py @@ -1,28 +1,20 @@ """Re-apply the non-BISAC nonfiction reset now that no old code is running. -Release N repaired these subjects: startup task -``2026_09_14_reclassify_non_bisac_nonfiction_subjects`` reset ``checked=False`` -on BISAC subjects stored as nonfiction because an unresolvable code fell -through the ruleset catch-all, and chained the re-score behind it. - -That repair is exposed for as long as any old code is still running. Anything -reaching Subject.assign_to_genre before the re-score lands consumes the reset, -and code running the superseded rules re-stamps checked=True with the same -wrong value -- silently, and with nothing to revisit the subject afterwards. -The deploy stops the Celery workers before the migrate step, so they are safe, -but the web containers are recycled after it and Fargate deployments are not -governed by that playbook at all. - -This task exists to close that off. Running it a release later means no old -code is live anywhere, so the reset it applies cannot be consumed under the old -rules. It is idempotent: if the release N repair took, this finds nothing to do -and the run is a no-op. - -Re-scoring is left to the nightly classify_unchecked_subjects. There is no need -to dispatch it here, because by now there is no old code to lose the reset to. - -This is the same shape as the null-audience repair, where startup task -2026_06_17 re-ran what 2026_05_12 had dispatched a release earlier. +Startup task ``2026_09_14_reclassify_non_bisac_nonfiction_subjects`` repaired +these subjects a release ago, but its reset was exposed to any old code still +live -- web containers are recycled after the migrate step, and Fargate is not +governed by that playbook at all. Old code reaching ``Subject.assign_to_genre`` +consumes the reset and re-stamps ``checked=True`` with the same wrong value; +see that task's docstring for the detail. A release later nothing old is +running anywhere, so the same reset is safe to apply again. + +It recomputes which subjects the classifier disagrees with rather than +replaying a stored list, so if the first repair took this is a no-op. +Re-scoring is left to the nightly ``classify_unchecked_subjects``; the release +N task chained it only because it was racing old code. + +This mirrors the null-audience repair, where ``2026_06_17`` re-ran what +``2026_05_12`` had dispatched a release earlier. TODO: Remove the whole repair once this has run on all deployments (PP-5129): this task, the release N startup task, ``reset_non_bisac_nonfiction_subjects`` From bef4f14ad49f631a19cdb6364ccf704df9dc0fa0 Mon Sep 17 00:00:00 2001 From: Daniel Bernstein Date: Fri, 2 Oct 2026 12:09:41 -0700 Subject: [PATCH 5/5] Re-date the task key and cover the real startup_tasks directory The key comes from the filename, so 2026_09_14_reapply sorted ahead of the 2026_09_14_reclassify repair it re-applies. Nothing has recorded the key yet, so re-dating it to the merge date is free and puts the two in deploy order. Also cover startup_tasks/ for the first time: every existing test points discovery at tmp_path, so a shipped task that fails to import would go green. Discovery skips a module it cannot import, so the new test compares the discovered keys against the files on disk rather than trusting what came back. Co-Authored-By: Claude Opus 5 --- ..._02_reapply_non_bisac_nonfiction_reset.py} | 11 +++--- tests/manager/scripts/test_startup.py | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) rename startup_tasks/{2026_09_14_reapply_non_bisac_nonfiction_reset.py => 2026_10_02_reapply_non_bisac_nonfiction_reset.py} (76%) diff --git a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py b/startup_tasks/2026_10_02_reapply_non_bisac_nonfiction_reset.py similarity index 76% rename from startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py rename to startup_tasks/2026_10_02_reapply_non_bisac_nonfiction_reset.py index d706ab9342..7ab2b66d05 100644 --- a/startup_tasks/2026_09_14_reapply_non_bisac_nonfiction_reset.py +++ b/startup_tasks/2026_10_02_reapply_non_bisac_nonfiction_reset.py @@ -2,11 +2,12 @@ Startup task ``2026_09_14_reclassify_non_bisac_nonfiction_subjects`` repaired these subjects a release ago, but its reset was exposed to any old code still -live -- web containers are recycled after the migrate step, and Fargate is not -governed by that playbook at all. Old code reaching ``Subject.assign_to_genre`` -consumes the reset and re-stamps ``checked=True`` with the same wrong value; -see that task's docstring for the detail. A release later nothing old is -running anywhere, so the same reset is safe to apply again. +live: hosting-playbook's ``helpers/migrate.yml`` recycles the web containers +after the migrate step, and does not govern Fargate deployments at all. Old +code reaching ``Subject.assign_to_genre`` consumes the reset and re-stamps +``checked=True`` with the same wrong value; see that task's docstring for the +detail. A release later nothing old is running anywhere, so the same reset is +safe to apply again. It recomputes which subjects the classifier disagrees with rather than replaying a stored list, so if the first repair took this is a no-op. diff --git a/tests/manager/scripts/test_startup.py b/tests/manager/scripts/test_startup.py index d9d357062f..190521f56a 100644 --- a/tests/manager/scripts/test_startup.py +++ b/tests/manager/scripts/test_startup.py @@ -15,7 +15,9 @@ from palace.util.datetime_helpers import utc_now +from palace.manager.celery.tasks.work import reset_non_bisac_nonfiction_subjects from palace.manager.scripts.startup import ( + STARTUP_TASKS_DIR, _slugify, create_startup_task, discover_startup_tasks, @@ -94,6 +96,42 @@ def test_discover_nonexistent_directory( assert result == {} assert "does not exist" in caplog.text + def test_discover_shipped_startup_tasks(self) -> None: + """Every task we actually ship imports and exposes a usable run(). + + The other tests here point discovery at `tmp_path`, so nothing else + imports `startup_tasks/` -- a task that fails to load would ship green. + Discovery logs and skips a module it cannot import, so compare against + the files on disk rather than just checking what came back. + """ + expected = { + path.stem + for path in STARTUP_TASKS_DIR.glob("*.py") + if not path.stem.startswith("_") + } + + result = discover_startup_tasks(STARTUP_TASKS_DIR) + + assert expected, f"No startup tasks found in {STARTUP_TASKS_DIR}." + assert set(result) == expected + assert all(callable(run) for run in result.values()) + + def test_reapply_non_bisac_nonfiction_reset(self) -> None: + """The re-apply task queues the reset, and only the reset. + + Unlike the release N task it deliberately does not chain the re-score. + + TODO: Remove with the rest of the repair (PP-5129). + """ + run = discover_startup_tasks(STARTUP_TASKS_DIR)[ + "2026_10_02_reapply_non_bisac_nonfiction_reset" + ] + + signature = run(MagicMock(), MagicMock(), logging.getLogger()) + + assert isinstance(signature, Signature) + assert signature.task == reset_non_bisac_nonfiction_subjects.name + class RunStartupTasksFixture: def __init__(self, mock_discover_startup_tasks) -> None: