Commit Graph
3 Commits
Author SHA1 Message Date
bvandeusenandClaude Opus 5 a4bdbcaca4 test: import the placement task module so its names reach celery.tasks (4246)
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 3s
Build images / build-agent (push) Successful in 6s
Build images / build-ml (push) Successful in 6s
CI / frontend-build (push) Successful in 22s
CI / backend-lint-and-test (push) Successful in 32s
Build images / sign-extension (push) Successful in 4s
Build images / build-web (push) Successful in 5s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
CI / integration (push) Successful in 2m26s
Three registration assertions failed: a task name only enters `celery.tasks`
when its module is imported, and nothing in the test process imported
`library_placement`. The API routes import it lazily inside the handlers, and
the registration test runs before any handler test triggers that.

`include=[...]` is what gets the module imported in a real WORKER, so
production registration was never in question — the test was asserting
something only observable after an import it never performed.

test_tasks_admin already carries the convention verbatim
(`import backend.app.tasks.admin  # noqa: F401 — register tasks`); I wrote the
assertion from what I meant instead of copying the idiom next to it. Same
mistake shape as the commit-vs-flush bounce one commit ago: the pattern was
already in the suite both times.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
2026-09-21 14:21:20 -04:00
bvandeusenandClaude Opus 5 db63377754 test: placement API tests must commit, not flush (4246)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 2s
Build images / build-agent (push) Successful in 8s
Build images / build-ml (push) Successful in 8s
Build images / build-web (push) Successful in 6s
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 37s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
CI / integration (push) Failing after 2m14s
Five of the new tests failed in CI: every one that created a
LibraryPlacementRun and then read it back through the client. The ones that
touched no rows passed.

A flush stays inside the test's own transaction, and the app under test runs
on a separate session and connection — so the endpoint queried a database
where the row did not exist yet and got its 404 / empty list honestly.
`_seed_runs` in test_api_system_backup already commits for this reason; the
idiom was there to copy and I did not look first.

Recorded in the helper's docstring rather than just fixed, since the next
person writing a create-then-fetch API test will reach for flush too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
2026-09-21 14:15:52 -04:00
bvandeusenandClaude Opus 5 abe449b4f2 feat: placement reconciler tasks + API (4246, slice 3b)
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 29s
CI / backend-lint-and-test (push) Successful in 1m1s
Build images / build-web (push) Successful in 1m34s
Build images / smoke-web (push) Skipped
Build images / build-ml (push) Successful in 2m26s
Build images / promote (push) Skipped
CI / integration (push) Failing after 3m5s
Three Celery tasks wrapping the 3a service, and the endpoints that drive
them. Routed to `maintenance_long` alongside backups: 33k renames on NFS have
no business in the quick lane where the self-healing sweeps live (the
2026-06-07 starvation).

A durability bug in 3a, found by thinking about what a crash costs rather
than by a failing test: `apply_run` wrote its ledger only at the end, so a
worker dying at row 30,000 of 33,789 would have taken the undo information
for the first 29,999 with it — and that ledger is the ONLY record of where
those files came from. It now persists every 200 moves. Two things fell out
of writing that:

- `_persist` reassigns `run.moves`, so the loop had to snapshot the plan
  first rather than iterate the attribute it rewrites.
- the reassignment is itself load-bearing: SQLAlchemy does not track in-place
  mutation of a JSONB list, so an `.append()` alone would never reach the
  database and the ledger would have stayed silently empty.

Re-running a partially-applied plan is safe — the moved rows no longer match
their `from` and refuse as "row moved since planning" — but `apply_placement`
deliberately has NO autoretry: re-entering a half-applied plan should be the
operator's call after reading what happened, not the queue's.

Endpoints gate on run state as well as the service does, so a stray POST
cannot re-apply an applied run. The list response omits `moves` (an applied
whole-library run carries tens of thousands of entries); the detail endpoint
includes them, because that detail IS the preview read before agreeing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
2026-09-21 14:13:25 -04:00