Correction-batch prerequisites: batch lineage and annotation provenance (G3, G4) - #311
Merged
Merged
Conversation
…2 and 3 The two facts a correction batch needs, recorded before anything creates one — because the alternative is discovering at that point that recording them takes a migration. **`batch.parent_batch_id`** (audit G4): which batch this one was cut from. A lineage fact, set at creation and never afterwards. NULL means *not a correction of anything*, which is true of every batch that exists — so migration 2 backfills nothing, and that is a complete answer rather than a shortcut. **`annotation.job_id`** (audit G3): which round of work produced this label. An annotation hangs off its `asset_id` and nothing else, so the batch id travelled only on a transient event and "which round made this box" had no answer anywhere. **The backfill is honest about what it cannot know.** An annotation whose asset belongs to exactly one job can be attributed with certainty; one whose asset is carried by two cannot be attributed at all, *because the schema never recorded which*. So the `UPDATE` sets a value only where the count is one and leaves the rest NULL. Writing "the first job we found" would put a confident wrong answer where an honest absent one belongs, and no reader downstream could tell which it had. It is also guarded on `job_id IS NULL`, so a re-run cannot overwrite an attribution a service has since written. **Neither column is a foreign key, and neither had a choice.** Both arrive by `ALTER TABLE`, which cannot express a key the way `create_all` does, and neither table can be rebuilt to escape that: `batch` carries `batch_asset` children whose `ON DELETE CASCADE` a `DROP TABLE` would follow silently, and `annotation` is not empty in any workspace that has ever been annotated. A rebuild is available only for a table that is childless *and* provably empty. `asset.source_id` is the precedent and records the same debt: a future delete of a parent has to clear these itself and say so. The chain exists again, so `test_a_fresh_database_and_a_migrated_one_have_the_same_schema` comes back with it — the comparison that catches a column declared in the wrong position, which nothing else in the suite would notice because each creation path is internally consistent. It builds its generation-1 file by *dropping* the two columns rather than by walking a current one backwards: there are no downgrade paths, and inventing one here would be inventing the thing under test. Dropping works for exactly the reason the columns are shaped as they are — SQLite refuses to drop a column carrying a key.
`job_id` is stamped by the service exactly as `schema_version` is, and for the same reason: the service knows which round this is and the caller does not get to claim otherwise. A field a client could set and never observe is a lie in the API. Without it the column would only ever hold what the migration could reconstruct, and every label written from now on would be as unattributable as the ambiguous ones. **`update` stamps the replacing job, while `asset_id` keeps the stored one — and the two go opposite ways on purpose.** `asset_id` answers *what this label is on*, which an edit must not silently move: relocating a label is a delete and an add. `job_id` answers *which round produced the label as it now stands*, and a replacement is a thing this round produced. Preserving the original would make the field mean "first written in", which is a different and weaker fact — it goes stale the moment a correction round edits.
…them `BatchOut.parent_batch_id` and `AnnotationOut.job_id`, on all four surfaces — REST, `visionset.wire`, MCP and the CLI's `--json`. Neither appears on an input model: both are stamped by the service, so a field a caller could set would be one it never observes. **The two nulls do not mean the same thing, and the models say so.** `parent_batch_id: null` is complete — a batch either was cut from another or was not. `job_id: null` is genuinely unknown: a label written before the column existed whose asset belonged to more than one job could not be attributed, and guessing would have been worse than admitting it. **A new server field is not automatically additive here, and this is the proof.** `@visionset/annotator`'s `parseAnnotation` checks the key set **exactly** — that is what #73 built it for — so a payload carrying a field the TS mirror omits is *refused outright*, not ignored. `job_id` therefore had to land in three places at once: `AnnotationOut`, the annotator's `Annotation`, and `ui-core`'s `WireAnnotation`. It is read-only in all three and absent from `AnnotationCreate` and `AnnotationUpdate`, on the `schema_version` precedent. `draftAnnotation` mints it `null`: the engine takes a document, not a workflow, and has no idea which job it is being driven inside — a client claiming provenance the service overwrites would be worse than an honest absence. `tests/fixtures/samples.py` carries a populated value for both, because that module holds *fully* populated instances on purpose: a sample with `None` where a value belongs lets that field's projection go unchecked.
JArmandoAnaya
enabled auto-merge (squash)
August 4, 2026 21:51
This was referenced Aug 5, 2026
JArmandoAnaya
added a commit
that referenced
this pull request
Aug 21, 2026
…ce (G3, G4) (#311) * feat(kernel): batch lineage and annotation provenance, as migrations 2 and 3 The two facts a correction batch needs, recorded before anything creates one — because the alternative is discovering at that point that recording them takes a migration. **`batch.parent_batch_id`** (audit G4): which batch this one was cut from. A lineage fact, set at creation and never afterwards. NULL means *not a correction of anything*, which is true of every batch that exists — so migration 2 backfills nothing, and that is a complete answer rather than a shortcut. **`annotation.job_id`** (audit G3): which round of work produced this label. An annotation hangs off its `asset_id` and nothing else, so the batch id travelled only on a transient event and "which round made this box" had no answer anywhere. **The backfill is honest about what it cannot know.** An annotation whose asset belongs to exactly one job can be attributed with certainty; one whose asset is carried by two cannot be attributed at all, *because the schema never recorded which*. So the `UPDATE` sets a value only where the count is one and leaves the rest NULL. Writing "the first job we found" would put a confident wrong answer where an honest absent one belongs, and no reader downstream could tell which it had. It is also guarded on `job_id IS NULL`, so a re-run cannot overwrite an attribution a service has since written. **Neither column is a foreign key, and neither had a choice.** Both arrive by `ALTER TABLE`, which cannot express a key the way `create_all` does, and neither table can be rebuilt to escape that: `batch` carries `batch_asset` children whose `ON DELETE CASCADE` a `DROP TABLE` would follow silently, and `annotation` is not empty in any workspace that has ever been annotated. A rebuild is available only for a table that is childless *and* provably empty. `asset.source_id` is the precedent and records the same debt: a future delete of a parent has to clear these itself and say so. The chain exists again, so `test_a_fresh_database_and_a_migrated_one_have_the_same_schema` comes back with it — the comparison that catches a column declared in the wrong position, which nothing else in the suite would notice because each creation path is internally consistent. It builds its generation-1 file by *dropping* the two columns rather than by walking a current one backwards: there are no downgrade paths, and inventing one here would be inventing the thing under test. Dropping works for exactly the reason the columns are shaped as they are — SQLite refuses to drop a column carrying a key. * feat(kernel): a label records the round that wrote it `job_id` is stamped by the service exactly as `schema_version` is, and for the same reason: the service knows which round this is and the caller does not get to claim otherwise. A field a client could set and never observe is a lie in the API. Without it the column would only ever hold what the migration could reconstruct, and every label written from now on would be as unattributable as the ambiguous ones. **`update` stamps the replacing job, while `asset_id` keeps the stored one — and the two go opposite ways on purpose.** `asset_id` answers *what this label is on*, which an edit must not silently move: relocating a label is a delete and an add. `job_id` answers *which round produced the label as it now stands*, and a replacement is a thing this round produced. Preserving the original would make the field mean "first written in", which is a different and weaker fact — it goes stale the moment a correction round edits. * feat(wire): both facts travel, and the annotator's mirror moves with them `BatchOut.parent_batch_id` and `AnnotationOut.job_id`, on all four surfaces — REST, `visionset.wire`, MCP and the CLI's `--json`. Neither appears on an input model: both are stamped by the service, so a field a caller could set would be one it never observes. **The two nulls do not mean the same thing, and the models say so.** `parent_batch_id: null` is complete — a batch either was cut from another or was not. `job_id: null` is genuinely unknown: a label written before the column existed whose asset belonged to more than one job could not be attributed, and guessing would have been worse than admitting it. **A new server field is not automatically additive here, and this is the proof.** `@visionset/annotator`'s `parseAnnotation` checks the key set **exactly** — that is what #73 built it for — so a payload carrying a field the TS mirror omits is *refused outright*, not ignored. `job_id` therefore had to land in three places at once: `AnnotationOut`, the annotator's `Annotation`, and `ui-core`'s `WireAnnotation`. It is read-only in all three and absent from `AnnotationCreate` and `AnnotationUpdate`, on the `schema_version` precedent. `draftAnnotation` mints it `null`: the engine takes a document, not a workflow, and has no idea which job it is being driven inside — a client claiming provenance the service overwrites would be worse than an honest absence. `tests/fixtures/samples.py` carries a populated value for both, because that module holds *fully* populated instances on purpose: a sample with `None` where a value belongs lets that field's projection go unchecked.
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.
Task 8 of the 2026-08 checkpoint audit remediation. Closes G4 (F21) and G3 (F20). The first migrations since the chain was collapsed —
FORMAT_VERSION1 → 3.Two facts, recorded before anything needs them
batch.parent_batch_idannotation.job_idThey land now rather than with the feature because the alternative is discovering at that point that recording them takes a migration.
The backfill is honest about what it cannot know
An annotation records only its
asset_id; a job records which assets it carries. So a label whose asset belongs to exactly one job can be attributed with certainty, and one whose asset is carried by two cannot be attributed at all — because the schema never recorded which. TheUPDATEsets a value only where the count is one.Writing "the first job we found" would put a confident wrong answer where an honest absent one belongs, and nothing downstream could tell which it had. It is also guarded on
job_id IS NULL, so a re-run cannot overwrite an attribution a service has since written — which matters because migration 1 iscreate_allof today's metadata, so a fresh database already carries the column and runs the migration anyway.Neither column is a foreign key, and neither had a choice
Both arrive by
ALTER TABLE, which cannot express a key the waycreate_alldoes, and neither table can be rebuilt to escape that:batchcarriesbatch_assetchildren whoseON DELETE CASCADEaDROP TABLEwould follow silently underPRAGMA foreign_keys = ON;annotationis not empty in any workspace that has ever been annotated.A rebuild is available only for a table that is childless and provably empty.
asset.source_idis the precedent, and both new columns record the same debt: a future delete of a parent has to clear them itself and say so.The fresh-vs-migrated test comes back
test_a_fresh_database_and_a_migrated_one_have_the_same_schemais the comparison that catches a column declared in the wrong position — nothing else in the suite would notice, because each creation path is internally consistent. It was reduced to a tautology when the chain collapsed; a second generation restores it.It builds its generation-1 file by dropping the two columns rather than by walking a current one backwards: there are no downgrade paths, and inventing one here would be inventing the thing under test. Dropping works for exactly the reason the columns are shaped as they are — SQLite refuses to drop a column carrying a key.
The service stamps, and
updatestamps the replacing jobjob_idis stamped exactly asschema_versionis. Without it the column would only ever hold what the migration could reconstruct.On
update,job_idandasset_iddeliberately go opposite ways.asset_idkeeps the stored value, because relocating a label is a delete and an add rather than an edit.job_idtakes the replacing job, because it answers which round produced the label as it now stands — preserving the original would make it mean "first written in", which goes stale the moment a correction round edits.This is a fact about who wrote a label, not a policy about which label wins in the trunk. G5 (trunk supersession) is untouched and remains the halt condition it was.
A new server field is not automatically additive — and this is the proof
@visionset/annotator'sparseAnnotationchecks the key set exactly (that is what #73 built it for), so a payload carrying a field the TS mirror omits is refused outright, not ignored.job_idhad to land in three places at once:AnnotationOut, the annotator'sAnnotation, andui-core'sWireAnnotation.draftAnnotationmints itnull— the engine takes a document, not a workflow, and has no idea which job it is being driven inside.Decisions taken autonomously
job_idon the wire read model only — absent fromAnnotationCreate/AnnotationUpdate, on theschema_versionprecedent.updatestamps rather than preserves (above).samples.pycarries populated values for both, because that module holds fully populated instances on purpose: aNonewhere a value belongs lets that projection go unchecked.Found, not fixed
annotation.job_idis unindexed. Nothing queries by it; the one future caller (labels of a round) would go through the job's assets.Test plan
All four suites:
bash scripts/check.sh— green (python, vitest 489, annotator 764, all linters, import-linter, openapi + generated-client + wire-fixture + mcp drift).CI=1 npx playwright test— 176 green.CI=1 npx playwright test -c playwright.cycle.config.ts— green.New: 5 migration tests (fresh-vs-migrated equivalence; the backfill attributing a single-job label; leaving an ambiguous one alone; not overwriting on a second run; lineage starting null), 4 annotation-service tests (stamped on add, caller cannot claim otherwise, stamped on update while
asset_idis preserved, survives a store round-trip), 3 batch-lineage tests, 3 wire-serialization tests._DECLARED_TAILSgains both tables — the column-order guard the migrations module's rules exist for, and the one that turns from tautology back into a real check now that a second generation exists.