From 69fc1f0d487952c62731f57f44f4cfc76ee6621c Mon Sep 17 00:00:00 2001 From: Therealdk8890 <35633053+Therealdk8890@users.noreply.github.com> Date: Sat, 18 Jul 2026 03:17:25 -0500 Subject: [PATCH] fix: normalize TraceRun events to ascending sequence at construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors DProvenanceKit PR #73. TraceEvent's own docstring declares sequence the authoritative causal order, but TraceRun kept whatever list order the caller assembled: the in-memory temporal query evaluator (after/before) used list order — diverging from the SQL backend, which orders by MIN(sequence), and from TRACE_SPEC_v1 — and the diff engine manufactured spurious added/removed pairs for hand-assembled out-of-order runs. align() already sorted internally, so alignment was correct but inconsistent with the other two surfaces. TraceRun now sorts events by sequence in __post_init__ (Python's sort is stable, so duplicate sequences keep the caller's relative order). Store load paths already delivered sorted lists and are unaffected. New test module pins the sort, the stable tie-break, verbatim preservation of sorted input, SQL-consistent temporal queries on unsorted input, and diff assembly-order invariance. --- dprovenancekit/query.py | 20 ++++++ tests/test_tracerun_normalization.py | 92 ++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 tests/test_tracerun_normalization.py diff --git a/dprovenancekit/query.py b/dprovenancekit/query.py index bdf997b..963021d 100644 --- a/dprovenancekit/query.py +++ b/dprovenancekit/query.py @@ -22,10 +22,30 @@ @dataclass(frozen=True) class TraceRun: + """A finished run's events in ascending ``sequence`` order. + + ``sequence`` is the authoritative causal order (see ``TraceEvent``); the + constructor normalizes whatever list it is given (ties keep the caller's + relative order), so two runs holding the same events query, diff, align, and + gate identically whether they were loaded from a store or assembled by hand. + This mirrors the Swift ``TraceRun`` initializer, and keeps the in-memory + temporal query evaluator consistent with the SQL backend, which already + orders by ``MIN(sequence)``. + """ + run_id: uuid.UUID context_id: str events: List[TraceEvent] + def __post_init__(self) -> None: + events = list(self.events) + if any( + events[i].sequence > events[i + 1].sequence + for i in range(len(events) - 1) + ): + events.sort(key=lambda e: e.sequence) # sorted() is stable in Python + object.__setattr__(self, "events", events) + # MARK: - AST nodes -------------------------------------------------------------- diff --git a/tests/test_tracerun_normalization.py b/tests/test_tracerun_normalization.py new file mode 100644 index 0000000..2733a4f --- /dev/null +++ b/tests/test_tracerun_normalization.py @@ -0,0 +1,92 @@ +"""Ports TraceRunNormalizationTests. + +Pins the ``TraceRun`` construction invariant: ``events`` is always in ascending +``sequence`` order (ties keep the caller's relative order), so causal analysis can +never depend on the order a caller assembled the list. Before this, the in-memory +temporal query evaluator (``after``/``before``) used list order — diverging from +both the SQL backend (``MIN(sequence)``) and TRACE_SPEC_v1 — and the diff engine +manufactured spurious added/removed pairs for hand-assembled out-of-order runs. +""" + +from __future__ import annotations + +import uuid +from dataclasses import dataclass + +from dprovenancekit import ( + TraceDiffEngine, + TraceEvent, + TracePriority, + TraceQueryDSL, + TraceableEvent, + TraceRun, +) + + +@dataclass(frozen=True) +class Step(TraceableEvent): + name: str + body: str = "b" + + @property + def type_identifier(self) -> str: + return self.name + + @property + def priority(self) -> TracePriority: + return TracePriority.CRITICAL + + +def _ev(seq: int, name: str, body: str = "b") -> TraceEvent: + return TraceEvent( + run_id=uuid.UUID("40000000-0000-0000-0000-000000000001"), + context_id="ctx", + engine_name="e", + schema_version=1, + sequence=seq, + span_id=None, + parent_span_id=None, + payload=Step(name=name, body=body), + ) + + +def _run(events) -> TraceRun: + return TraceRun(run_id=uuid.uuid4(), context_id="ctx", events=events) + + +def test_out_of_order_list_is_sorted_by_sequence(): + run = _run([_ev(2, "C"), _ev(0, "A"), _ev(1, "B")]) + assert [e.sequence for e in run.events] == [0, 1, 2] + assert [e.payload.name for e in run.events] == ["A", "B", "C"] + + +def test_already_sorted_list_is_preserved_verbatim(): + events = [_ev(0, "A"), _ev(1, "B"), _ev(2, "C")] + run = _run(events) + assert [e.id for e in run.events] == [e.id for e in events] + + +def test_duplicate_sequences_keep_assembly_order(): + first, second = _ev(1, "dup", "first"), _ev(1, "dup", "second") + run = _run([_ev(2, "tail"), first, second, _ev(0, "head")]) + assert [e.sequence for e in run.events] == [0, 1, 1, 2] + assert run.events[1].payload.body == "first" + assert run.events[2].payload.body == "second" + + +def test_temporal_query_matches_sql_semantics_on_unsorted_input(): + # Causally A(0) runs before B(1); the list was assembled B-first. The + # in-memory evaluator used list order here and answered False, while the + # SQL backend (MIN(sequence)) answered True for the same run. + run = _run([_ev(1, "B"), _ev(0, "A")]) + query = TraceQueryDSL().requiring_followed_by("A", followed_by="B") + assert query.ast.evaluate(run) is True + + +def test_diff_is_assembly_order_invariant(): + sorted_run = _run([_ev(0, "authorize", "a"), _ev(1, "charge", "c")]) + shuffled = _run([_ev(1, "charge", "c"), _ev(0, "authorize", "a")]) + result = TraceDiffEngine().diff(base=sorted_run, comparison=shuffled) + assert result.is_identical, ( + "same events, same causal order — assembly order must not manufacture changes" + )