rbrooks/Circa
Watch
1
0
Fork
You've already forked Circa
 0
Code Issues 66 Pull requests 1 Projects Releases 2 Packages Wiki Activity Actions

Implement the background worker runtime and job state machine #2

New issue
Closed
opened 2026-07-28 04:52:46 +00:00 by claude-bot · 3 comments
claude-bot commented 2026-07-28 04:52:46 +00:00
Copy link

Context

POST /api/photos/{photo_id}/evidence/ai-rerun and .../ocr-rerun already create
BackgroundJob rows, and the Jobs page renders them, but backend/app/workers/
contains only __init__.py. Nothing ever claims a job, so every rerun sits in
queued forever. This is the largest functional gap left after v0.1.0.

Scope

A standalone worker process that durably moves jobs through the JobStatus
state machine (queued -> running -> success | failed | cancelled),
with a handler registry keyed by JobType.

Implementation notes

  • Entry point: python -m app.workers.runner (add a console script in pyproject.toml).
  • Claim loop: poll for the oldest queued job, then claim it with a conditional
    UPDATE ... WHERE status = 'queued' so a second worker cannot take the same row.
    SQLite has no SELECT ... FOR UPDATE; rely on the affected-rowcount of the
    conditional update to decide whether the claim succeeded.
  • On claim: set status = running, stamp started_at.
  • On success: set status = success, stamp finished_at, write result_summary.
  • On failure: capture the exception into error_message, increment retry_count,
    and return the job to queued while retry_count < max_retries; otherwise set
    failed. Use backoff between retries so a persistently failing job cannot spin.
  • Handler registry: dict[JobType, Callable] so ai_analysis, ocr, and the later
    duplicate_scan / constraint_rebuild / backup / export types register uniformly.
  • payload is a JSON Text column — define and document the per-JobType payload shape.
  • idempotency_key is already unique=True; the enqueue path must not raise a
    500 on a duplicate enqueue, it should return the existing job.
  • Each handler runs in its own DB transaction. Partial work must never commit —
    a crashed handler leaves the job retryable with no half-written evidence.
  • Recover orphans on startup: any job left running with no live worker (e.g. after a
    crash) should be requeued or failed rather than being stuck forever.

Done when

  • A queued job is claimed, executed, and reaches a terminal state without manual intervention
  • Two concurrent workers never execute the same job
  • A handler that raises leaves the job retryable and writes no partial data
  • A job exceeding max_retries lands in failed with a readable error_message
  • Worker startup and shutdown are documented in the README

References

  • backend/app/workers/ (currently empty)
  • backend/app/api/routes/jobs.py
  • backend/app/models/models.py (BackgroundJob, JobType, JobStatus)
  • docs/circa-phase1-plan.md section 5.7, section 9

Depends on: nothing. Blocks the AI and OCR job handlers.

## Context `POST /api/photos/{photo_id}/evidence/ai-rerun` and `.../ocr-rerun` already create `BackgroundJob` rows, and the Jobs page renders them, but `backend/app/workers/` contains only `__init__.py`. Nothing ever claims a job, so every rerun sits in `queued` forever. This is the largest functional gap left after v0.1.0. ## Scope A standalone worker process that durably moves jobs through the `JobStatus` state machine (`queued` -> `running` -> `success` | `failed` | `cancelled`), with a handler registry keyed by `JobType`. ## Implementation notes - Entry point: `python -m app.workers.runner` (add a console script in `pyproject.toml`). - Claim loop: poll for the oldest `queued` job, then claim it with a conditional `UPDATE ... WHERE status = 'queued'` so a second worker cannot take the same row. SQLite has no `SELECT ... FOR UPDATE`; rely on the affected-rowcount of the conditional update to decide whether the claim succeeded. - On claim: set `status = running`, stamp `started_at`. - On success: set `status = success`, stamp `finished_at`, write `result_summary`. - On failure: capture the exception into `error_message`, increment `retry_count`, and return the job to `queued` while `retry_count < max_retries`; otherwise set `failed`. Use backoff between retries so a persistently failing job cannot spin. - Handler registry: `dict[JobType, Callable]` so `ai_analysis`, `ocr`, and the later `duplicate_scan` / `constraint_rebuild` / `backup` / `export` types register uniformly. - `payload` is a JSON `Text` column — define and document the per-`JobType` payload shape. - `idempotency_key` is already `unique=True`; the enqueue path must not raise a 500 on a duplicate enqueue, it should return the existing job. - Each handler runs in its own DB transaction. Partial work must never commit — a crashed handler leaves the job retryable with no half-written evidence. - Recover orphans on startup: any job left `running` with no live worker (e.g. after a crash) should be requeued or failed rather than being stuck forever. ## Done when - [ ] A queued job is claimed, executed, and reaches a terminal state without manual intervention - [ ] Two concurrent workers never execute the same job - [ ] A handler that raises leaves the job retryable and writes no partial data - [ ] A job exceeding `max_retries` lands in `failed` with a readable `error_message` - [ ] Worker startup and shutdown are documented in the README ## References - `backend/app/workers/` (currently empty) - `backend/app/api/routes/jobs.py` - `backend/app/models/models.py` (`BackgroundJob`, `JobType`, `JobStatus`) - `docs/circa-phase1-plan.md` section 5.7, section 9 Depends on: nothing. Blocks the AI and OCR job handlers.
claude-bot added this to the v0.2.0 milestone 2026-07-28 04:52:46 +00:00
claude-bot commented 2026-07-28 06:07:50 +00:00
Author
Copy link

Amended by the audit of 2026-07-28.

Two additions to this issue's requirements:

  1. The idempotency requirement described here is incomplete. This issue says a duplicate
    enqueue "must not raise a 500," describing the concurrent case. The actual bug (#82) is the
    terminal-predecessor case: idempotency_key is f"{job_type}:{photo_id}" with a unique
    constraint, and the short-circuit only covers queued/running. So the second-ever rerun of a
    type on a photo — the normal "the AI got it wrong, try again" — raises IntegrityError.

  2. SQLite is not configured for a concurrent writer (#87). synchronous=FULL, deferred BEGIN
    (so a read-then-write transaction racing a worker commit fails immediately with
    SQLITE_BUSY_SNAPSHOT — the busy handler cannot rescue a lock upgrade), and an implicit 5 s
    busy timeout. #87 should land before this issue.

Also relevant: #65 proposes moving Pillow EXIF parsing into this worker under resource limits, so a
malicious image cannot take down the API process.

**Amended by the audit of 2026-07-28.** Two additions to this issue's requirements: 1. **The idempotency requirement described here is incomplete.** This issue says a duplicate enqueue "must not raise a 500," describing the *concurrent* case. The actual bug (#82) is the **terminal-predecessor** case: `idempotency_key` is `f"{job_type}:{photo_id}"` with a unique constraint, and the short-circuit only covers `queued`/`running`. So the second-ever rerun of a type on a photo — the normal "the AI got it wrong, try again" — raises `IntegrityError`. 2. **SQLite is not configured for a concurrent writer** (#87). `synchronous=FULL`, deferred `BEGIN` (so a read-then-write transaction racing a worker commit fails *immediately* with `SQLITE_BUSY_SNAPSHOT` — the busy handler cannot rescue a lock upgrade), and an implicit 5 s busy timeout. #87 should land before this issue. Also relevant: #65 proposes moving Pillow EXIF parsing into this worker under resource limits, so a malicious image cannot take down the API process.
claude-bot commented 2026-08-01 20:37:59 +00:00
Author
Copy link

Done in b9b3a58, on top of #87 (4db9ad9), which this needed first.

Done when

  • A queued job is claimed, executed, and reaches a terminal state without manual intervention
  • Two concurrent workers never execute the same job — tested both ways: three workers racing for one job (exactly one execution, two get None), and four workers draining twelve jobs (every job handled exactly once)
  • A handler that raises leaves the job retryable and writes no partial data
  • A job exceeding max_retries lands in failed with a readable error_message
  • Worker startup and shutdown are documented in the README

python -m app.workers.runner, or the circa-worker console script. --once and --max-jobs N for scripted use. 27 tests in tests/test_worker_runtime.py; full suite 542 passed.

The design decision worth recording: a claim is a lease.

The issue says to "recover orphans on startup: any job left running with no live worker should be requeued or failed". Doing that literally — requeue every running row when a worker starts — needs no schema and no heartbeat, and is wrong as soon as there are two workers: the second one starting cannot tell "abandoned" from "in progress on the other machine", so it takes the first one's jobs and runs them again. The requirement one line above it, that two workers never execute the same job, is what rules it out.

So a claim carries an owner and a deadline (migration 008: claimed_by, lease_expires_at, run_after). The claiming worker renews from a heartbeat thread while it works, so a long handler is never at risk; the lease bounds recovery time, not job duration. A running row whose lease has lapsed is genuinely abandoned and anyone may take it.

Two consequences that are deliberate:

  • Reclaiming charges a retry. A job that kills the process running it — a decoder bug on a malformed scan, an allocation the box cannot satisfy — would otherwise be reclaimed forever and take out each worker in turn. Charging the attempt makes it land in failed like anything else that cannot be completed.
  • A worker that lost its claim discards its result. If the heartbeat fails to renew, or the row is no longer ours at finalize time, the outcome is thrown away rather than written over the new owner's. Tested.

The handler contract, which is where #87's rule went.

A run is three transactions — claim, handle, finalize — and the handler gets a way to open a transaction (ctx.write()), not an open one. Handing over a live session would make holding the write lock across a network call the path of least resistance, and on SQLite that blocks every reviewer save for the length of the call. TestTheWorkerHoldsNoLockWhileHandling asserts it rather than trusting the docstring: another connection writes and commits while a handler is running, and the test fails if it has to wait.

Use one ctx.write() block per handler. The runner guarantees a raising handler leaves the job retryable, but it cannot un-commit a block that already succeeded.

On the amendments in the comment above

  1. The terminal-predecessor idempotency bug was #82, fixed separately — idempotency_key now carries an attempt number, so sequential reruns are distinct while concurrent ones collide on the unique constraint. Nothing further needed here; the enqueue path already returns the winner rather than raising.
  2. #87 landed first, as suggested. Its comment records one correction to its own diagnosis.
  3. #65's remaining half — moving Pillow into the worker — is not done here. It now has somewhere to go: it wants a handler registered against a job type, which is the app/workers/handlers/ package this adds.

Two kinds of failure. Anything raised is retried with exponential backoff (30 s doubling, capped at 15 min) until max_retries; PermanentJobError is not retried at all. The distinction is whether trying again could plausibly give a different answer — a timeout could, a photo that does not exist could not.

What an AI or OCR rerun does today. It is claimed and fails with "No handler is registered for job type ai_analysis", because #3 and #4 have not landed. That is deliberate and I would like it reviewed as a choice: it is a true statement that shows up on the Jobs page, where the previous behaviour was an indefinite queued that said nothing. app/workers/handlers/__init__.py is where #3 and #4 register.

Verified against a real database with the real process, not only in tests: two jobs enqueued the way the API enqueues them, both claimed, both refused with the message above, claims released, both recorded in the audit ledger as job_failed with retryable: false.

Unrelated, found on the way. The #87 commit turned CI red on lint. My local ruff is 0.4.4 and CI pins 0.16.0, and they disagree on whether alembic is first- or third-party — there is an alembic/ directory in the project, so the inferred answer depends on whether the linter resolves submodule paths against it. My ruff "fixed" two files into a state CI rejected. known-third-party = ["alembic"] is now stated in pyproject.toml and both versions agree; I verified against 0.16.0 in a throwaway venv rather than assuming.

Done in b9b3a58, on top of #87 (4db9ad9), which this needed first. **Done when** - [x] A queued job is claimed, executed, and reaches a terminal state without manual intervention - [x] Two concurrent workers never execute the same job — tested both ways: three workers racing for one job (exactly one execution, two get `None`), and four workers draining twelve jobs (every job handled exactly once) - [x] A handler that raises leaves the job retryable and writes no partial data - [x] A job exceeding `max_retries` lands in `failed` with a readable `error_message` - [x] Worker startup and shutdown are documented in the README `python -m app.workers.runner`, or the `circa-worker` console script. `--once` and `--max-jobs N` for scripted use. 27 tests in `tests/test_worker_runtime.py`; full suite 542 passed. **The design decision worth recording: a claim is a lease.** The issue says to "recover orphans on startup: any job left `running` with no live worker should be requeued or failed". Doing that literally — requeue every `running` row when a worker starts — needs no schema and no heartbeat, and is wrong as soon as there are two workers: the second one starting cannot tell "abandoned" from "in progress on the other machine", so it takes the first one's jobs and runs them again. The requirement one line above it, that two workers never execute the same job, is what rules it out. So a claim carries an owner and a deadline (migration `008`: `claimed_by`, `lease_expires_at`, `run_after`). The claiming worker renews from a heartbeat thread while it works, so a long handler is never at risk; the lease bounds *recovery time*, not job duration. A `running` row whose lease has lapsed is genuinely abandoned and anyone may take it. Two consequences that are deliberate: - **Reclaiming charges a retry.** A job that kills the process running it — a decoder bug on a malformed scan, an allocation the box cannot satisfy — would otherwise be reclaimed forever and take out each worker in turn. Charging the attempt makes it land in `failed` like anything else that cannot be completed. - **A worker that lost its claim discards its result.** If the heartbeat fails to renew, or the row is no longer ours at finalize time, the outcome is thrown away rather than written over the new owner's. Tested. **The handler contract, which is where #87's rule went.** A run is three transactions — claim, handle, finalize — and the handler gets a way to *open* a transaction (`ctx.write()`), not an open one. Handing over a live session would make holding the write lock across a network call the path of least resistance, and on SQLite that blocks every reviewer save for the length of the call. `TestTheWorkerHoldsNoLockWhileHandling` asserts it rather than trusting the docstring: another connection writes and commits while a handler is running, and the test fails if it has to wait. Use one `ctx.write()` block per handler. The runner guarantees a raising handler leaves the job retryable, but it cannot un-commit a block that already succeeded. **On the amendments in the comment above** 1. The terminal-predecessor idempotency bug was #82, fixed separately — `idempotency_key` now carries an attempt number, so sequential reruns are distinct while concurrent ones collide on the unique constraint. Nothing further needed here; the enqueue path already returns the winner rather than raising. 2. #87 landed first, as suggested. Its comment records one correction to its own diagnosis. 3. #65's remaining half — moving Pillow into the worker — is not done here. It now has somewhere to go: it wants a handler registered against a job type, which is the `app/workers/handlers/` package this adds. **Two kinds of failure.** Anything raised is retried with exponential backoff (30 s doubling, capped at 15 min) until `max_retries`; `PermanentJobError` is not retried at all. The distinction is whether trying again could plausibly give a different answer — a timeout could, a photo that does not exist could not. **What an AI or OCR rerun does today.** It is claimed and fails with "No handler is registered for job type ai_analysis", because #3 and #4 have not landed. That is deliberate and I would like it reviewed as a choice: it is a true statement that shows up on the Jobs page, where the previous behaviour was an indefinite `queued` that said nothing. `app/workers/handlers/__init__.py` is where #3 and #4 register. Verified against a real database with the real process, not only in tests: two jobs enqueued the way the API enqueues them, both claimed, both refused with the message above, claims released, both recorded in the audit ledger as `job_failed` with `retryable: false`. **Unrelated, found on the way.** The #87 commit turned CI red on lint. My local ruff is 0.4.4 and CI pins 0.16.0, and they disagree on whether `alembic` is first- or third-party — there is an `alembic/` directory in the project, so the inferred answer depends on whether the linter resolves submodule paths against it. My ruff "fixed" two files into a state CI rejected. `known-third-party = ["alembic"]` is now stated in `pyproject.toml` and both versions agree; I verified against 0.16.0 in a throwaway venv rather than assuming.
claude-bot commented 2026-08-01 20:44:29 +00:00
Author
Copy link

Follow-up in cba2a0d, after CI went green on b9b3a58.

Reviewing my own work I found the loop treated two different failures the same way. A handler raising is the job's failure and run_one deals with it. An exception from the machinery — the claim unwritable, the database unreachable, and #87 made the busy timeout finite — propagated out of run_forever and ended the process. Defensible for a one-shot script, wrong for a daemon: the queue then stops silently until somebody notices, which for a background process can be days. It now backs off exponentially and carries on, so a fault that is not transient reads as a repeating log line rather than as silence. RunReport.errors counts them, and there is a test that makes the claim fail twice and checks the worker still drains the queue.

Also: the startup reclaim sweep did not record that it had swept, so the first idle poll swept again immediately.

543 tests pass; lint clean against the pinned ruff.

Follow-up in cba2a0d, after CI went green on b9b3a58. Reviewing my own work I found the loop treated two different failures the same way. A handler raising is the *job's* failure and `run_one` deals with it. An exception from the machinery — the claim unwritable, the database unreachable, and #87 made the busy timeout finite — propagated out of `run_forever` and ended the process. Defensible for a one-shot script, wrong for a daemon: the queue then stops silently until somebody notices, which for a background process can be days. It now backs off exponentially and carries on, so a fault that is not transient reads as a repeating log line rather than as silence. `RunReport.errors` counts them, and there is a test that makes the claim fail twice and checks the worker still drains the queue. Also: the startup reclaim sweep did not record that it had swept, so the first idle poll swept again immediately. 543 tests pass; lint clean against the pinned ruff.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
renovate/lock-file-maintenance
v0.3.3
v0.3.2
v0.3.1
v0.3.0
v0.2.0
Labels
Clear labels   
ai
AI / OCR evidence backends

  
backend
Backend / API / services

  
bug
Something is not working

  
docs
Documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
frontend
React UI

  
help wanted
Need some help

  
infra
CI, deployment, tooling

  
invalid
Something is wrong

  
question
More information is needed

  
security
Auth, authorization, data safety

  
testing
Automated tests and QA

  
wontfix
This won't be fixed

  
worker
Background jobs and worker runtime

No labels
ai
backend
bug
docs
duplicate
enhancement
frontend
help wanted
infra
invalid
question
security
testing
wontfix
worker
Milestone
Clear milestone
No items
No milestone
v0.2.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Circa#2
No description provided.