No projection rebuild path; add one as the test oracle #79
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: HIGH (architectural keystone)
The problem
Photocarries denormalized projection fields whose docstring contract is that they "must bederivable from canonical history." Nothing can actually derive them.
AuditEventType.projection_rebuiltexists atmodels/models.py:131with zero references.services/ingest.py:129-149,in
services/projections.py, and inPhotoRepository.apply_decision_projection.photo.statusdirectly, and the supersederoute flips
is_activewith no projection or audit context.So any bug — like the supersede route's half-update — drifts silently and permanently, with no
way to detect or repair it.
Fix
Write
rebuild_photo_projection(photo_id)that recomputes status,current_decision_id, and theeffective date fields purely from decision and evidence history. Define the fold explicitly:
pending→needs_reviewtransitionEmit
projection_rebuilt. Expose it as a CLI or admin action, per-photo and collection-wide.Route all status changes through
projections.pyso the rules exist in one place.Why this is the highest-leverage artifact in the test milestone
It turns "projections cannot drift" from a docstring into a property test: after any sequence of
operations, stored projection must equal rebuild output. It is simultaneously the repair tool for
drift that can already happen today.
Done when
rebuild_photo_projectionexists and is the single definition of the foldprojection_rebuiltReferences
backend/app/services/projections.pybackend/app/repositories/photos.py:80-101backend/app/services/ingest.py:129-149backend/app/models/models.py:131Related: #6 (projection tests) should use this as its oracle.
Done in
5b021a2. All five "done when" items covered, 34 tests inbackend/tests/test_projection_rebuild.py.The fold, stated once (
projections.fold):pending→needs_review, and only evidence a reviewer could act onThat middle clause needed a definition the issue didn't specify. I used reliability above
low, which is what makes it agree with #74: a scan-date upper bound is recorded atlowand is genuinely useful to #19, but it is not a reason to put a photo in someone's queue. Filename (medium), manual and believed EXIF (high) all clear the bar.The structural choice that matters: every write path recomputes through the fold rather than patching fields incrementally. An incremental update that happens to match a separately-written rebuild is exactly the arrangement that drifts — sharing the implementation means they can't disagree. Ingest no longer decides status at all; it used to write it twice, once per evidence kind, which is how the rules ended up in three modules.
Tie-breaking is on
(created_at, id), notcreated_atalone. Two decisions can share a timestamp, and if "latest" were ambiguous the rebuild could disagree with itself between runs, which would make drift detection worthless.Writing the property test found real drift immediately — the supersede bypass this issue names. Retiring a photo's only actionable evidence left it in
needs_reviewwith nothing left to review. That route now recomputes.python -m app.cli.rebuild_projections [--dry-run] [--photo ID] [--collection ID]is both detector and repair. It emitsprojection_rebuiltwith both sides, because if the stored value disagreed with history, the record of what it used to claim is the only way to work out which decisions were made while the archive was showing something false. It exits non-zero on drift, so it works as a scheduled integrity check — drift is a defect and should page someone rather than scroll past in a log.Two tests guard the structure rather than the behaviour: one greps
ingest.pyand the route modules forphoto.status =so the rules cannot quietly move back out of the service, and one assertsapply_decision_projectionis gone rather than merely unused.Not applicable: "Running rebuild over the existing database reports no drift" — there is no existing database, since the previous deployment was removed. The property test covers the equivalent guarantee over synthesised histories.
#6 can now use this as its oracle, as you noted it should.