Skip to content

Validate agreement-target ownership on every identification write path #1378

Description

@mihow

Summary

The single-occurrence identification endpoint accepts any identification or classification in the database as an "agreement target", with no check that the target actually belongs to the occurrence being identified. The bulk endpoint added in #1371 does validate this per item. The two write paths therefore enforce different contracts for the same two fields (agreed_with_identification / agreed_with_prediction). This issue proposes moving the ownership check to a single place — the Identification model — so every write path enforces it consistently.

Background

agreed_with_identification and agreed_with_prediction are provenance fields: they record that a human identification agrees with an earlier identification or a machine prediction. Identification.save() does not branch on them, so an incorrect agreement target does not corrupt an occurrence's determination or score. The effect of a wrong target is a nonsense provenance link (an identification claiming to agree with a prediction of some unrelated occurrence), not a broken determination. That is why this is a data-quality consistency issue rather than an urgent correctness bug.

Current behavior

  • Bulk endpoint (ami/main/api/views.py, IdentificationViewSet.bulk → _validate_item): rejects an agreement target whose occurrence differs from the item's occurrence, returning a per-item error. It also handles the case where a classification's detection has been deleted, so the link back to an occurrence is missing.
  • Single-occurrence endpoint (ami/main/api/serializers.py, IdentificationSerializer): declares both fields as plain PrimaryKeyRelatedField, which only checks that the referenced row exists. Any identification or classification in the system is accepted, regardless of which occurrence it belongs to.

Proposed change

Move the "agreement target must belong to the occurrence being identified" rule onto the model, for example in Identification.clean() (or enforced in save()), so it holds regardless of entry point — the single-occurrence endpoint, the bulk endpoint, the Django admin, and the shell. The bulk endpoint's per-item validation can then defer to the shared rule rather than duplicating it.

Why this is a separate change, not part of #1371

Enforcing the rule on the model changes the single-occurrence endpoint's contract: it would begin returning a 400 for payloads it currently accepts. That is the correct behavior, but it is a contract change that deserves its own review, its own tests, and a heads-up to the frontend, rather than riding along on a bulk-endpoint PR. #1371 keeps the bulk endpoint strict and documents the asymmetry deliberately; this issue tracks closing the gap on the other side.

Acceptance criteria

  • The ownership rule lives in one place on the Identification model and is covered by tests.
  • The single-occurrence endpoint rejects an agreement target from a different occurrence (400), with a test.
  • The bulk endpoint's per-item check defers to the shared rule (no duplicated logic), and its existing tests still pass.
  • The nullable-detection case (a classification whose detection was deleted) is handled by the shared rule, matching the current bulk behavior.

Follow-up to #1371.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions