Skip to content

fix: normalize TraceRun events to ascending sequence at construction - #56

Merged
Therealdk8890 merged 1 commit into
mainfrom
fix/tracerun-sequence-normalization
Jul 18, 2026
Merged

fix: normalize TraceRun events to ascending sequence at construction#56
Therealdk8890 merged 1 commit into
mainfrom
fix/tracerun-sequence-normalization

Conversation

@Therealdk8890

Copy link
Copy Markdown
Owner

Mirrors DProvenanceKit PR #73 (merged), keeping the two SDKs' ordering contracts identical.

Problem

TraceEvent's docstring declares sequence the authoritative causal order, but TraceRun kept the caller's list order, and the three analysis surfaces disagreed about what order means:

  • align() sorted internally (alignment_engine.py:59-64) — correct;
  • in-memory temporal queries (after/before, query.py AfterNode/BeforeNode) used list order — diverging from the SQL backend, which orders by MIN(sequence) (TRACE_SPEC_v1 §temporal operators), so the same query on the same hand-built run answered differently depending on the store;
  • diff() walked list order — a hand-assembled out-of-order run produced spurious added/removed pairs against its store-loaded twin.

Store load paths always delivered sequence-ordered lists, so this bit only hand-constructed runs (e.g. events decoded from a consumer's own JSON).

Fix

TraceRun.__post_init__ normalizes events to ascending sequence (stable sort — duplicate sequences keep assembly order), mirroring the Swift initializer. All three surfaces now share one ordering basis.

Verification

  • New tests/test_tracerun_normalization.py (5 tests, ports TraceRunNormalizationTests): sort, stable tie-break, verbatim sorted input, SQL-consistent temporal query on unsorted input, diff invariance.
  • Full suite: 484 passed, 4 skipped — conformance/vector parity unaffected (vectors assign sequence = list index, so they were already normalized).

Follow-up (both repos)

Conformance vectors carry no explicit sequence field — both harnesses assign sequence = index, so vectors are structurally blind to ordering bugs like this one. A vector class with explicit out-of-order sequences needs a coordinated spec addition in both repos; tracked for a separate PR.

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.
@Therealdk8890
Therealdk8890 merged commit f8d6e9c into main Jul 18, 2026
9 checks passed
@Therealdk8890
Therealdk8890 deleted the fix/tracerun-sequence-normalization branch July 18, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant