migrate: V_epsilon target + NDI-layer stimulus_bath → bath assembly#829
Merged
Conversation
Adds ndi.migrate.internal.stimulusBathToBath: the session-aware build that the
per-document converter defers (did2:convert:needsSessionContext). It assembles
a complete `bath` (pharmacological_manipulation) from a legacy stimulus_bath:
- mixture (concentrations) + location from the document,
- subject_id resolved from the stimulator element's subject,
- time_reference = epoch_bounded_reference on the stimulator's epoch
(the stimulator is the time referent; no other connection kept).
Session/element lookups come via a `resolver` struct
(.subjectOfElement / .epochClockOfElement) that ndi.migrate.local will supply
from the open session's element graph.
STATUS: scaffold (authored without local MATLAB; assembly shape complete, the
resolver wiring + the ndi.migrate.local second pass over deferred docs are TODO).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ABEeDJ83r9Hq9PaiSije8o
…#782) Finish the NDI side of the stimulus_bath -> bath migration so the context-dependent deferrals the per-document DID converter emits are actually resolved during ndi.migrate.local: - ndi.migrate.internal.bodyResolver(bodies): index the open body set and answer the session-aware questions the per-document converter cannot -- subjectOfElement (following underlying_element_id up the derivation chain) and epochClockOfElement (element_epoch, then epochclocktimes, then the conventional dev_local_time default). - ndi.migrate.local: add a TargetVersion option (default 'V_delta', preserving the tested behaviour and V_delta.sqlite destination). Under 'V_epsilon' it writes V_epsilon.sqlite, threads TargetVersion into v1_to_v2, and runs a guarded second pass (resolveDeferred) that re-assembles every needsSessionContext deferral via bodyResolver + stimulusBathToBath, folds the assembled V_epsilon bodies back through v1_to_v2 (which short-circuits already-target bodies to pad+validate), and moves the successes from quarantine into migrated. The second pass is wrapped in try/catch so a resolver failure degrades to leaving the deferrals quarantined rather than failing the whole run. Verified in NDI CI (no local MATLAB). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ABEeDJ83r9Hq9PaiSije8o
The full NDI suite (run-tests.yml) installs the stable released did-matlab, whose did2.convert.v1_to_v2 predates the TargetVersion name-value option (that option lives on the unmerged DID-matlab #149 branch). Passing 'TargetVersion' unconditionally therefore broke every TestMigrateLocal case -- including the default V_delta path -- with an arguments-block rejection. Forward TargetVersion only when it differs from the default 'V_delta', so the default migration call stays byte-identical to before and remains compatible with the stable did-matlab. A 'V_epsilon' run adds the name-value and requires the newer did-matlab, which it needs regardless. The V_epsilon second pass and the per-target destination path were already guarded behind strcmp(TargetVersion,'V_epsilon'), so the default path is fully restored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ABEeDJ83r9Hq9PaiSije8o
…#782) The context-dependent stimulus_bath -> bath migration had only synthetic unit tests; nothing proved the assembled bath VALIDATES against V_epsilon or that the resolver reads the right subject/epoch from a live body set. - TestMigrateLocalEpsilon: builds a complete v1 session on disk (stimulator element + element_epoch + stimulus_bath) and runs ndi.migrate.local with TargetVersion='V_epsilon', Validate=true, asserting the stimulus_bath is NOT left deferred, a `bath` + `epoch_bounded_reference` are produced, and the bath carries the stimulator's subject_id + a time_reference and the reference carries the epoch + clock. Gated on mksqlite + NDI_TEST_EPSILON + DID_SCHEMA_PATH so it skips everywhere the V_epsilon environment is absent. - test-epsilon-migrate.yml: assembles that environment (matching DID-matlab E branch via head_ref + assembled V_epsilon schema set + NDI_TEST_EPSILON) and runs the gated test -- neither run-tests.yml (stable DID, V_delta) nor test-vnext.yml (V_delta) can. - stimulusBathToBath.parseMixture: guarantee >= 1 mixture record (blank fallback). pharmacological_manipulation.mixture is mustBeNonEmpty, so an empty mixture_table would otherwise make the assembled bath fail validation -- exactly the kind of gap this end-to-end test exists to find. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ABEeDJ83r9Hq9PaiSije8o
Contributor
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## Vnext #829 +/- ##
==========================================
- Coverage 23.59% 23.43% -0.17%
==========================================
Files 715 717 +2
Lines 31177 31404 +227
==========================================
+ Hits 7357 7360 +3
- Misses 23820 24044 +224 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Vnext NDI unit test results672 tests +672 655 ✅ +655 5m 52s ⏱️ + 5m 52s For more details on these errors, see this check. Results for commit aa162f6. ± Comparison against base commit eefcaa0. ♻️ This comment has been updated with latest results. |
The first real run of TestMigrateLocalEpsilon (issue #782) surfaced the bug it was written to catch: ndi.migrate.local feeds bodyResolver the v1 bodies straight from did2.convert.readers.sqliteV1, which returns RAW JSON char strings, not structs. bodyResolver's isstruct-based index therefore indexed nothing, subjectOfElement could not resolve, and resolveDeferred's per-entry catch silently left the stimulus_bath deferred (no bath, no epoch_bounded_reference produced). Normalise the body set at the top of bodyResolver: decode any char/string entry via jsondecode, keep scalar structs. Handles both the fresh read path (cellstr from the readers) and the idempotent re-run path (structs from toStruct). This is the gap end-to-end CI verification exists to find. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ABEeDJ83r9Hq9PaiSije8o
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the NDI-layer half of the Brainstorm-E (
V_epsilon) migration: theper-document DID converter (DID-matlab #149) defers the classes it cannot
complete from a single document, and this PR resolves them where the whole
session/element graph is in hand.
The companion DID-matlab work splits
treatment→ manipulation tiers andontology_table_row→ observation tiers (1 → N) per document, but astimulus_bath→bathcannot be completed per-document: a manipulationmust be emitted complete, and the resulting
bathneeds two things onlyreachable by following the stimulator element — its subject and an
epoch_bounded_reference on the stimulator's epoch. The DID converter
therefore defers
stimulus_bathwithdid2:convert:needsSessionContext;this PR does the session-aware assembly.
What's here
ndi.migrate.internal.bodyResolver(bodies)— indexes the open bodyset and answers the questions a single document can't:
subjectOfElement(elementId)— the element'ssubject_id, followingunderlying_element_idup the derivation chain (cycle-guarded).epochClockOfElement(elementId, epochId)— the clocktype for theelement's epoch: the matching
element_epoch, then anyepochclocktimesfor the epoch (favouringdev_local_time), then theconventional
dev_local_timedefault.ndi.migrate.internal.stimulusBathToBath(scaffolded earlier) is nowactually invoked: it assembles the
epoch_bounded_reference+ thebath(
pharmacological_manipulation.mixture+bath.kind/location) from thelegacy
stimulus_bathusing the resolver.ndi.migrate.local— newTargetVersionoption:'V_delta'preserves the existing, tested behaviour and theV_delta.sqlitedestination — no change for current callers.'V_epsilon'writesV_epsilon.sqlite, threadsTargetVersionintov1_to_v2, and runs a guarded second pass (resolveDeferred) thatre-assembles every
needsSessionContextdeferral via the resolver, foldsthe assembled
V_epsilonbodies back throughv1_to_v2(whichshort-circuits already-target bodies to pad + validate, not re-migrate),
and moves the successes from quarantine into migrated. The pass is wrapped
in try/catch so a resolver failure degrades to leaving the deferrals
quarantined rather than failing the run.
Testing
No local MATLAB was available; correctness is vetted by CI. The DID side is
exercised by
did2.unittest.testMigratorsE(includingtestStimulusBathDefersToNdiLayerandtestAlreadyEpsilonBodyShortCircuits,which guards the fold-back short-circuit this PR relies on).
Pairs with DID-matlab #149 (the per-document
V_epsilonmigrators and theisAlreadyTarget/TargetVersionplumbing).🤖 Generated with Claude Code
Generated by Claude Code