Skip to content

redesigning Annotation and Document property handling #411

Closed

Description

@keighrim

mmif-python handles document and annotation properties through three property stores on Document (original, pending, ephemeral) and a view-level default mechanism on Annotation. Together they serve two goals:

  1. automatic recording of document-level metadata, such as the technical metadata of videos, as capital Annotation objects (automatic document-level Annotation recorder #226)
  2. factoring shared property values out into a view's contains metadata and redistributing them on load (handling of annotation props in contains metadata #396)

This "three-dicts" approach is now largely broken. It has accumulated ad-hoc patches over the years (#290, #299, #393, #396, #401, #408, among others), and each fix added a rule that the next change had to respect. The latest round of bugs, found while preparing apps for mmif-python 1.6.0, shows that the problems are structural rather than local. It is time to revisit the fundamental design and fix the root causes where possible.

Root causes

  1. One store, many meanings. _props_ephemeral holds values that differ in origin and lifetime: view-level contains defaults, values copied from capital Annotations onto documents, and computed start/end caches. Until canonicalize aliased property names instead of mirroring them #401 it also held property aliases. The precedence rules between them live in whichever code reads the store, and handling of annotation props in contains metadata #396 was one such collision.
  2. Serialization changes the object it serializes. serialize() generates capital Annotations into views, deletes factored values from annotation instances, and edits contains. As a result, annotation.properties means one thing before serialization and another after it. Since factor-out was generalized to all annotation types (improving shared-value property factoring #398) and turned on by default, every reader of annotation.properties has silently become a reader of incomplete data. serialize(sanitize=True), which clams-python always uses, calls serialize() a second time inside sanitize() to produce the JSON it validates against the MMIF schema. That inner call generates capital Annotations and factors out values again, on an object the outer call has already changed, so the inconsistency shows up even within a single app run.
  3. Pending document properties are never cleared, so the duplicate check in generate_capital_annotations() is the only guard against recording the same property more than once. That check has been fixed repeatedly and still fails when values have been factored out.
  4. The read-only promise does not hold. Input documents, and in principle every view an earlier app produced, should serialize unchanged however they are used in memory. Instead:
    • add_property writes text, mime, and location straight into the document, so the serialized input document changes.
    • Values that fail the simple-value check also bypass the pending store and are written into the document.
    • Factor-out runs over every view at serialization, so an earlier app's view is rewritten whenever its annotations share a value.
    • generate_capital_annotations() writes the current app's capital Annotation into the latest view that annotates the document, which can be an earlier app's view (it shouldn't, but current bug allows this scenario).
    • The add_property docstring states that existing properties cannot be overwritten.

Symptoms currently known (1.5.3 through 1.6.0):

  • video document lookups fail on TimeFrames whose document is set at view level
  • get_start/get_end fail on TimePoints whose timePoint was factored out
  • document properties shared by two or more documents are lost on reload
  • the same document properties are recorded twice on every app run
  • new_contain silently drops view-level defaults when the type is already declared
  • annotations added after a serialization inherit defaults they never had

Reader-side fixes for the first four are in progress as a stopgap for a 1.6.1 release. They do not address the causes above.

Plan

Investigate the best way to fix the root causes. This is the core mechanism behind much of mmif-python's "magic" groundwork: transparent property access, view-level defaults, and automatic metadata recording, all of which clams-python and every app rely on. It needs to be fixed as soon as possible.

The investigation should settle:

  • whether the original goals still hold, and which parts of the current behavior apps depend on
  • which constraints any fix must keep, such as MMIF output compatibility and the public property API
  • how far the fix can go within the 1.x line, and what would have to wait for 2.0

Activity

  1. keighrim commented on Nov 1, 2026

    @keighrim
    MemberAuthor

    #413 is open and removes the three-dicts approach, targeting 1.7.0. The output format stays at MMIF 1.2.0.

    Root causes

    Going through the four root causes in the issue body;

    1. _props_ephemeral mixing different values: I stopped copying contains defaults onto annotations. An annotation now keeps a pointer to its parent view and looks up view-level values when they're read, so get_property and in work for both levels, and .properties is only the raw stored data. _props_ephemeral is still there for values copied from capital Annotations and for the cached start/end. _props_original is removed.
    2. serialize() changing the object: it now builds the output once, adds capital records and factored values to that output, and validates that same output, so sanitize=True doesn't serialize twice anymore. This also made serialize(sanitize=True) faster on our largest test file (30,639 annotations), 5.35s --> 3.59s.
    3. pending properties never cleared: they still aren't, but it doesn't matter anymore because serialization writes nothing back to the object.
    4. read-only promise: only partly fixed, see the write mode and guardrail sections below.

    All six symptoms listed in the issue body are fixed.

    Write mode

    mmif-python can't tell what "this run" is by looking at a Mmif, so I added a write mode where the producer declares it;

    signature = View()
    self.sign_view(signature, refined)  # `ClamsApp.sign_view()`, in `annotate()`
    with mmif_obj.set_producer(signature) as producer:
        ...  # the app does its work, as usual
        producer.add_warnings(*caught)
        output = mmif_obj.serialize(pretty=True, sanitize=True)

    signature is a dummy/orphan View signed by the producer. Namely, while the mode is on,

    1. views that existed when the producer was set are written as they are in memory
    2. capital records go to the run's last view, or to a new signed view when the run made none
    3. factor-out, contains pruning, and empty-view removal touch the run's views only
    4. all views of the run share one run ID (the timestamp of its first view)
    5. warnings go to one signed warnings view, and EmptyOutputWarning moves here from clams-python

    One exception to (1); old MMIF files that use frameType are read as label, and that spelling is what gets written back. I think that's fine.

    Didn't add failure handling (__exit()__ ) since exceptions just propagate, and restifier and cli.py already have a good handle on errors.

    ClamsApp.annotate() will be the only caller, and _annotate() keeps working on the same Mmif object, so app code doesn't need any change.

    Guardrail

    Document.add_property now refuses to change location, text, or mime once they're set; a different value raises AttributeError, and the same value does nothing. The spec already makes the documents list read-only after initialization and mmif source fills it before the first app runs, so this check doesn't need to know about the write mode. It only covers add_property though, and doc.location = ... still goes through. The ownership-based version of this check is for 2.0 (there's a TODO at the check).

    Compatibility

    Existing code should keep working without the write mode. As a check, I ran the clams-python 1.8.1 code with its pin lifted against #413, and it passes its tests with DeprecationWarning ignored. We won't release that combination. The next clams-python release based on this change in mmif-python (probably 1.8.2 as a patch) moves annotate() to set_producer().

    So I ran some tests with real apps on the upcoming mmif-py 1.7.0 and clams-py 1.8.2, with no change in app code;

    1. app-swt-detection on a 28-min video, GPU, Python 3.10: same TimePoints, TimeFrames, and capital record as the released SDK, and about the same run time (32s vs. 33s). The only other difference is the order of labelset (because of set's random seed).
    2. app-spacy-wrapper on two whisper transcripts: same annotations as 1.5.3 once view-level values are resolved, and the two SDK versions can read each other's output.

    Outside the write mode, the old behavior stays but marked as deprecated.

    • generate_capital_annotations()
    • factor_out_shared_properties()
    • sanitize()
    • serialize() called with deprecated keywords
      • serialize(), in addition, raises a FutureWarning when it writes capital records,
  2. moved this from Todo to Done in infraon Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions