feat(dice-storage): Drivine-backed MetamodelVersionStore

[jimador]

Breaking changes: none. Additive — new classes, a new dice-metamodel dependency in dice-storage, and new Neo4j uniqueness constraints for hosts that adopt the store. No existing class or Cypher schema changed.

Stacked on #83 (feat/metamodel-versioning) — review that first; this goes ready-for-review once #83 lands. PR 2 of the metamodel train.

What's in:

• DrivineMetamodelVersionStore — Neo4j persistence for the MetamodelVersionStore contract: MERGE-on-natural-key idempotent saves, latestVersion/versionHistory/findVersion, corrupt-row resilience (a bad newest row hides only itself).
• Write order is a persisted monotonic per-schema sequence, not wall-clock time; a MetamodelVersion(schemaName, sequence) uniqueness constraint makes duplicate positions unstorable, so contention failures are loud and retryable. Hosts declare three constraints (documented in the class KDoc, design doc, and CHANGELOG).
• Property signatures serialize as deterministic sorted JSON (JVM Map.copyOf iteration order is randomized per run — unsorted encoding would rewrite unchanged stamps).
• Shared Neo4jTestContainer adopted by all @SpringBootTest ITs in the module; 21 version-store ITs including concurrent-save (one node survives at 48 savers) and mixed-timestamp ordering.

Tests: dice-storage 160/0/0, dice-metamodel 120/0/0, dice 1187/0/0. Reviewed by Codex (gpt-5.6-sol); ordering-contract finding fixed with the sequence + constraint design.

Next in stack: PR3 observed schema + diff contracts.

---

Amendment (prior-art adoption, EXPERIMENTAL): the store now persists what the #83 amendment declared.

• Alias round-trip: entityTypeAliases and per-signature aliases serialize only when non-empty and decode absent-as-empty, so an alias-free stamp writes byte-identical node properties to the pre-alias writer and old-shape rows read back cleanly through the strict hash-integrity recompute — pinned by a raw-Cypher old-row test, a both-alias-kinds round-trip, and a dropped-alias-map rejection test.
• Contract as well as implementation: InMemoryMetamodelVersionStore (promoted to dice-metamodel main sources, following the repo's InMemory* convention) and AbstractMetamodelVersionStoreContractTest run one suite against both backends.

Amendment tests: dice-storage 160/0/0, dice-metamodel 120/0/0. Gates: adversarial Claude Fable PASS (coalesce corner semantics, byte-identity, and the store-promotion deviation all verified), Codex (gpt-5.6-sol) clean. Mutation checks confirm each guard bites (naive SET, dropped alias serialization, reverted in-memory rules each fail their exact tests).

Review update (2d03ad0): stamp-provenance persistence is removed with the type (#83's review round): the coalesce assignments, the row-mapper provenance fields, the saveVersion contract clause, and the provenance tests. Aliases persistence, sequence ordering, the store promotion, and the old-shape readability test are untouched. Current tests: dice-storage 134, dice-metamodel 113.

---

Why ordering is a persisted sequence. Wall-clock time makes duplicate positions storable and silent. A per-schema monotonic sequence with a uniqueness constraint makes them unstorable, so contention surfaces as a loud retryable failure.

sequenceDiagram
    participant H as Host
    participant S as DrivineMetamodelVersionStore
    participant N as Neo4j
    H->>S: saveVersion(stamp)
    S->>N: next per-schema sequence
    S->>N: MERGE on natural key
    Note over N: uniqueness on schemaName plus sequence
    alt position already taken
        N-->>S: constraint violation
        S-->>H: loud, retryable
    else position free
        N-->>S: stored
        S-->>H: version at sequence n
    end

Loading

[igordayen]

@jimador - looks good, few comments to consider. this is first batch, manual review.

[igordayen]

@jimador - Claude review:

---

PR #84 Review — DrivineMetamodelVersionStore

Overall: The design is solid. Sequence counter, MERGE idempotence, integrity check on read, alias backward
compatibility, and corrupt-row resilience are all correctly designed and well-tested. Several issues
worth raising.

---

Design

1. open class DrivineMetamodelVersionStore — undocumented

The class is open with no explanation. If it's open for Spring CGLIB proxy generation (required for
@transactional to work without an interface), that should be stated in the KDoc. If the project uses the
kotlin-spring allopen compiler plugin, the open is redundant and should be removed. Either way this is
unclear.

2. (schemaName, contentHash) as natural key in findVersion (inherited from feat(metamodel): schema versioning core with per-type governance #83)

As noted on #83: contentHash alone is a SHA-256 of the full structural content — it's globally unique by
construction. findVersion(schemaName, contentHash) carrying schemaName is redundant. The store's ordering
queries legitimately need schemaName, but keyed lookup does not.

3. Contract test is too thin

AbstractMetamodelVersionStoreContractTest has only 2 tests — upsert idempotence and ordering. Missing
behavioral contracts that both backends must honour:

• findVersion returns null for an unknown hash
• versionHistory returns newest-first (not just "not oldest-first")
• Schema isolation: writes to schema A don't appear in schema B
• latestVersion returns null for an unknown schema

These are currently tested only in the Neo4j integration test, meaning an alternative implementation can
silently fail them.

4. No documented retry contract for sequence constraint violations

The KDoc acknowledges that a concurrent counter increment can fail with a constraint violation on
(schemaName, sequence). The caller "can retry" — but saveVersion just propagates the exception. There's no
retry at this layer, and the interface KDoc doesn't say callers must handle it. Document the expected
caller behaviour, or add a retry loop inside saveVersion with a bounded attempt count.

---

MetamodelRowMappers.kt

5. private val objectMapper = ObjectMapper() — bare Jackson at file scope

Igor's comment is valid. The project has EmbabelObjectMapperHolder for exactly this reason — neutral to
Jackson 2/3. A bare ObjectMapper() becomes a migration hazard. It should also be scoped inside
MetamodelVersionRowMapper rather than at file level; right now any future addition to the file can access
it silently.

6. Empty-string guards in deserializers are misleading

private fun deserializeList(serialized: String): List =
if (serialized.isEmpty()) emptyList()
else ...

An empty JSON list serializes as "[]", not "". The isEmpty() branch only fires if the node holds a literal
empty string, which shouldn't happen given the write path. These guards should either be removed (and an
exception thrown if the stored value is "", since that's corruption) or replaced with a check for "[]" /
"{}". As-is, they silently swallow an unexpected case.

7. filterIsInstance<Map<*, *>>() in readVersions drops non-map rows silently

persistenceManager.query(spec).filterIsInstance<Map<*, *>>().mapNotNull { row ->

Rows that aren't Map<*, *> are dropped without logging. If Drivine returns an unexpected type (node
object, etc.), the caller gets fewer results with no diagnostic. At minimum, log a warning when
filterIsInstance discards a row.

8. @Suppress("UNCHECKED_CAST") scope

In deserializeMapOfSignatureSets and deserializeMapOfLabelSets, @Suppress is on the function declaration
but the cast is on one specific line. Kotlin supports @Suppress on the exact expression — narrower scope
is safer.

---

InMemoryMetamodelVersionStore

9. at variable name

val at = saved.indexOfFirst { ... }
if (at < 0) saved += version else saved[at] = version

at is an index (-1 = not found). idx or existingIndex is clearer.

10. synchronized(saved) in versionHistory returns a filtered copy

override fun versionHistory(schemaName: String): List =
synchronized(saved) { saved.filter { it.schemaName == schemaName }.reversed() }

This is correct — the lock covers the filter and the copy before returning. However, latestVersion calls
versionHistory which re-acquires the same lock; on the JVM, synchronized is reentrant so this is safe, but
it's worth noting it traverses the list twice for latestVersion. Calling saved.lastOrNull { ... }
directly inside a lock would be O(1) vs O(n).

---

Igor's comment assessment

┌──────────────────────────────────────┬──────────────────────────────────────────────────────────────┐
│               Comment                │                            Valid?                            │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ Package placement for                │ Debatable — the PR follows existing InMemory* convention     │
│ InMemoryMetamodelVersionStore        │                                                              │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ at variable name                     │ Yes — see #9 above                                           │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ Reflection/beanWrapper for bindMap   │ No — the KDoc explicitly justifies explicit field binding:   │
│                                      │ property renames must not silently change the persisted key  │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ Multiline string syntax              │ Yes — style, minor                                           │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ EmbabelObjectMapperHolder            │ Yes — see #5 above                                           │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ Separate file for serialization      │ Reasonable, but MetamodelRowMappers.kt is already a          │
│ utils                                │ dedicated file; splitting further would split tightly        │
│                                      │ coupled read/write pairs                                     │
├──────────────────────────────────────┼──────────────────────────────────────────────────────────────┤
│ @ActiveProfile("dice-test") on       │ Valid Spring convention question — worth checking if other   │
│ contract IT                          │ ITs in the module use it                                     │
└──────────────────────────────────────┴──────────────────────────────────────────────────────────────┘

[igordayen]

@jimador - looks good, thank you

[jimador]

Answered in 3efcc7c and 80fb127 on this branch. The stack above is restacked on it and the full reactor is green at the top.

1. open is gone. The allopen spring preset only opens a class that carries a Spring annotation itself, and this one annotated its methods alone, which is why it needed the manual open. It now carries @Transactional at class level like the other Drivine stores, with the reads keeping readOnly = true.
2. Answered on feat(metamodel): schema versioning core with per-type governance #83: the MetamodelVersionStore KDoc explains why the name stays in the key. The hash excludes the schema name on purpose, so two schemas with one shape share a hash, and history is per schema.
3. The contract suite now covers a missed findVersion under both a wrong hash and a wrong schema, newest-first ordering across three saves, schema isolation, and latestVersion on an empty schema. Both backends run it.
4. Documented on the interface and the store. saveVersion propagates the constraint violation and does not retry: the failure ends the Neo4j transaction, so a retry needs a new one, which only the caller can open. The write is idempotent, so the retry is always safe, and the drift check re-stamps on every pass anyway.
5. Stays Jackson 2 for now. dice is Jackson 2 throughout and EmbabelObjectMapperHolder is Jackson 3, so that swap belongs to Upgrade to jackson3 #76. The helpers stay file-private: feat(dice-storage): Drivine drift-report store and observed-schema source #87 adds DriftReportRowMapper to the same file and shares them, and nothing outside the file can reach the mapper.
6. Guards removed. An empty collection is written as [] or {}, so "" is corruption and now fails the read like any other bad JSON, with a test.
7. A row that is not a map is logged at warn with its runtime class and skipped, the same path a corrupt row takes.
8. The casts are gone. The maps are read through TypeReference, so there is nothing left to suppress.
9. Done in 0618b95.
10. latestVersion answers with lastOrNull under the lock.