feat(kernel): IngestJob lifecycle — transition table, persisted progress, per-file report, resume (#19) - #92
Merged
Conversation
…ess, per-file report, resume Closes #19. #20 shipped a working pipeline and left the job record thin: the row went in already at `running`, terminal states were written straight onto it, and the per-file report lived only in memory. This adds the lifecycle around that path rather than rewriting it. - `INGEST_TRANSITIONS` in `domain/ingest.py`, with the kernel's first backward edge (`failed -> running`) and a deliberate absence (`running -> running`). - `processed` / `total` written once per item, so `IngestService.get` answers "where is it now" from any connection. `total` is NULL for a clip, because `VideoMetadata` carries no frame count by design. - The per-file report as a JSON column, next to the fatal `error` it is not. - `IngestService.resume(job_id)`: a redo on the same row, into the batch the first attempt was headed for. - Migration 9 `ingest_job_progress`; FORMAT_VERSION 8 -> 9. - `require_move` promoted from `job_service` to `domain/transitions.py`, and `BatchService`'s second spelling of it removed.
3 tasks
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 21, 2026
…ess, per-file report, resume (#19) (#92) * feat(kernel): IngestJob lifecycle — transition table, persisted progress, per-file report, resume Closes #19. #20 shipped a working pipeline and left the job record thin: the row went in already at `running`, terminal states were written straight onto it, and the per-file report lived only in memory. This adds the lifecycle around that path rather than rewriting it. - `INGEST_TRANSITIONS` in `domain/ingest.py`, with the kernel's first backward edge (`failed -> running`) and a deliberate absence (`running -> running`). - `processed` / `total` written once per item, so `IngestService.get` answers "where is it now" from any connection. `total` is NULL for a clip, because `VideoMetadata` carries no frame count by design. - The per-file report as a JSON column, next to the fatal `error` it is not. - `IngestService.resume(job_id)`: a redo on the same row, into the batch the first attempt was headed for. - Migration 9 `ingest_job_progress`; FORMAT_VERSION 8 -> 9. - `require_move` promoted from `job_service` to `domain/transitions.py`, and `BatchService`'s second spelling of it removed. * docs(ingest): state the resume precondition as the table does, not as 'failed only'
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #19.
#20 shipped a working pipeline and deliberately left the job record thin: the row went in already at
running, terminal states were written straight onto it, and the per-file report lived only in memory. Bothdomain/ingest.pyanddocs/ingest.mdsaid so in as many words. This adds the lifecycle around that path rather than rewriting it.Acceptance criteria
test_re_ingesting_one_source_creates_nothingalready asserted it foringest;test_resuming_creates_no_new_blobs_for_what_was_already_storednow asserts the same forresume, which is what makes a redo affordable.test_progress_is_visible_to_somebody_who_is_not_running_the_ingestwatches the row through a secondWorkspaceServiceopened on the same directory, not another call on the service doing the work. Only a separate connection can show the counters are committed as the run goes, which is the claim.test_a_run_records_which_files_failed_and_why(per-file, run still completes) andtest_a_fatal_cause_is_recorded_apart_from_the_per_file_report(theerrorcolumn, withfailuresempty).test_a_failed_run_keeps_the_progress_it_had_madecovers the position a failure stopped at.Design
Resume re-enters the same row.
INGEST_TRANSITIONSgets one backward edge,failed → running. The argument against reopening a batch does not carry over: a batch pins a schema version and its jobs are already cut against that pin, whereas nothing is pinned against an ingest run. A row per attempt would forkbatch_idand turnlist(source_id)into a list of retries. Refusing to resume a completed job then needs no new error — it is a move the table does not have, so it is an ordinaryInvalidTransition. No new error class in this PR.running → runningis deliberately absent, so a job stuck atrunningcannot be resumed. That state is a process that died without reporting anything; the remedy already exists (ingest again, which creates nothing), and overwriting the row would erase the only evidence of the crash.Counters are written once per item. An interval that suits five files and one that suits fifty thousand are different numbers and the service cannot know which it is looking at, so there is no cadence constant to guess. One small commit beside a decode-and-hash that costs an order of magnitude more. The progress writes are not a contradiction of the four-transaction rule: each opens and commits while nothing is being decoded — the hazard is a transaction held across the decode.
totalis NULL for a clip, and that is honest.VideoMetadatacarries no frame count by design (a guess for VFR; the number an ingest wants is what extraction produced), so a total there would be arithmetic presented as fact. A directory states its total before the first file, which is also what makes an empty one record0 of 0.The report is one JSON column, the way
source.videoholds a wholeVideoProvenance: read whole, never queried by field.INGEST_JOBSbecomes the eighth hand-written mapper pair.A job is now created
pendingand moved torunningby whoever picks it up. Today that is the same call and the state lasts microseconds — it is spelled out because it is the vocabulary a queue needs, and because adding it later would change what a stored row means. Every refusal still happens before the insert, so a fail-fast leaves no job row at all.The hazard this closes
Repository.updateissession.merge(to_row(entity))— a whole-row replace.ingest()capturedjobin its first transaction and re-used that object after the decode. The moment counters are written in between, that stale copy would have silently undone all of them on completion. So nothing carries anIngestJobacross the decode any more: onlyjob_idtravels, and every write re-reads the row inside its own transaction. That is whatrequire_job(uow, job_id)was made public for in #20.Migration 9 —
FORMAT_VERSIONis now 9ingest_jobgainsbatch_name,processed,total,failures. The plainest migration in the file, and the plainness is the point after 8: none of the four carries a foreign key, soALTERcan express all of them, and each has an honest value for a pre-#19 row (that run counted nothing and reported nothing). Nothing is refused and nothing is dropped.batch_nameearns its column rather than being convenience: a run that died during the decode reached no batch, so without it a resumed run would fall back to naming the batch after the source folder and quietly lose the name the caller asked for.A coverage gap worth the entry. Migration 8 rebuilt this table, so its column-order exemption expired the moment this build started writing rows — but the fresh-versus-migrated test walks back to generation 1, from where migration 8 re-creates the table whole (from
_tables, so including migration 9's columns) and migration 9 finds them present and does nothing. A migration whose only exercise is through an earlier rebuild is not exercised at all.test_migration_nine_alters_a_table_migration_eight_rebuiltsets a database to generation 8 and compares the realALTERpath against a fresh one. That baseline is produced by dropping the four columns rather than by retyping theCREATE TABLE, because these tests comparesqlite_mastertext and a hand-written one differs in whitespace — a false negative about this file rather than about the schema. That the drops are possible at all is migration 9's own argument restated.Migration 9 needs no undo in
_downgrade_to_version_one: its columns live oningest_job, which migration 8's undo rebuilds from scratch. That is now commented there so the next migration on this table does not assume the same.Also
require_movepromoted fromjob_service's module-private helper todomain/transitions.py, andBatchService's second spelling of the same sentence deleted — the standing "a gate two services need is promoted, not copied" rule. Message text is byte-identical, so every sweep still matches on"cannot become".tests/kernel/test_metadata_store.pyseeds the ingest job with a populatedfailurestuple, per that file's convention that a nested shape left empty leaves the round trip untested.SchemaService/LabelClassintest_ingest_service.pyfolded into aFixture.freeze()helper.docs/ingest.mdgains a lifecycle section and a progress section;docs/persistence.mdcarries the ledger,FORMAT_VERSION9, and the rebuild-exemption expiry.Checks
VERSIONstays0.0.1.dev0; no new dependency; no new error class; ingest has no route yet, soopenapi.jsonis unchanged.