[did2 #3] Flip NDI_DID2_NORMALIZE_ON_READ — always normalise on read#803
Open
stevevanhooser wants to merge 10 commits into
Open
[did2 #3] Flip NDI_DID2_NORMALIZE_ON_READ — always normalise on read#803stevevanhooser wants to merge 10 commits into
stevevanhooser wants to merge 10 commits into
Conversation
Closes #776. With the alias-table read augmentation (#779), write-time reconciliation (#780), query path translation (#781), and the issue-#801 follow-up that moved depends_on entry-key compat into ndi.document accessors plus ndi.compat.translateQueryPaths -- all landed on Vnext now -- every caller that still reads did_v1 field names is covered without polluting the body's depends_on struct-array schema. The env-var gate is therefore redundant. Per the cleanup checklist on issue #779 (comment 4493101768): 1. Delete the NDI_DID2_NORMALIZE_ON_READ gate in ndi.database.internal.applyReadNormalization: removed the normalizationGateOn() helper and the `if ~normalizationGateOn()` short-circuit, so every read body is routed through did2.convert.v1_to_v2. 2. Drop the OFF-state and gate-truthy/falsy tests from TestApplyReadNormalization: removed testGateOffWrapsBodyUnchanged, testGateTruthyValues, and the TestMethodSetup / TestMethodTeardown env-var save/restore plumbing. Remaining tests renamed from "testGateOn..." to drop the gate-state prefix. 3. Update the docstring on applyReadNormalization: dropped the "gated by env var" paragraph and cross-referenced the four alias-shim helpers (augmentRead, normalizeDependsOn, reconcileWrite, translateQueryPaths) that make this default-on behaviour safe. grep -rn NDI_DID2_NORMALIZE_ON_READ src/ tests/ returns nothing. Note: an earlier attempt to flip this gate (PR #800) was closed without merging because CI surfaced two compat-layer gaps the alias-table triad did not cover. Those were tracked in #801 and fully resolved by PR #802 + the cross-repo did-matlab #137/#138/ #139 chain. The compat layer is now sufficient.
Contributor
…epoch.clocks shape did-schema reverted the V_delta renames for element.name/type, daqreader_ndr.ndr_reader_string + ndi_daqreader_ndr_class, and daqmetadatareader.ndi_daqmetadatareader_class, so the compat shims added in #801 for those blocks become dead weight. Removed: - ndi.compat.fieldAliases row mapping daqmetadatareader.reader_class <-> .ndi_daqmetadatareader_class (the field never moves now; no alias needed) - ndi.database.fun.ndi_document2ndi_object: dropped the resolveReconstructorClass helper and the daqreader_ndr override. V_delta restored ndi_daqreader_ndr_class on the body, so the pre-#801 lookup of `<parent>.ndi_<parent>_class` works directly. - Companion test deletions in AugmentReadTest and FieldAliasesTest. For element_epoch the user elected to keep V_delta's new structural shape (array-of-records `clocks` with name/t0/t1 fields) over a revert: ndi.element/loadaddedepochs now reads clocks directly instead of splitting epoch_clock CSV and indexing into the parallel t0_t1 matrix. This is the one reader site that exercises the V_delta restructure; everything else is identity passthrough. These three changes plus the did-schema revert and the did-matlab migrator deletions close out the four CI failure categories surfaced by the gate flip in eac9161.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| else | ||
| obj_struct = getfield(ndi_document_obj.document_properties, obj_parent_string); | ||
| obj_string = resolveReconstructorClass(obj_parent_string, obj_struct); | ||
| obj_string = getfield(obj_struct,['ndi_' obj_parent_string '_class']); |
| ec{k} = ndi.time.clocktype(clock_types{k}); | ||
| t0_t1{k} = vlt.data.rowvec(potential_epochdocs{i}.document_properties.element_epoch.t0_t1(:,k)); | ||
| for k=1:numel(clocks_array) | ||
| ec{k} = ndi.time.clocktype(char(clocks_array(k).name)); |
| t0_t1{k} = vlt.data.rowvec(potential_epochdocs{i}.document_properties.element_epoch.t0_t1(:,k)); | ||
| for k=1:numel(clocks_array) | ||
| ec{k} = ndi.time.clocktype(char(clocks_array(k).name)); | ||
| t0_t1{k} = vlt.data.rowvec([clocks_array(k).t0, clocks_array(k).t1]); |
Two follow-ups to PR #141 (did-matlab moves schema_version from base to document_class) so the gate-on CI clears the remaining 13 structural failures. (1) demoNDI / demoNDIMock schema classname After flipping the did2 normalize gate, universalRenames snake-cases the doc's document_class.class_name ('demoNDI' -> 'demo_ndi'). The old did v1 validator (did.database/validate_doc_vs_schema:1228) compares the schema's `classname` field against the doc's class_name and threw `DID:Database:ValidationClassname` because the NDI-shipped demoNDI_schema.json still declared classname 'demoNDI'. V_delta convention is snake_case across the board (did-schema's V_delta/index.json registers this class as `demo_ndi`). The fix is local to NDI: update the inner classname (and property-block name) on demoNDI_schema.json -> 'demo_ndi', and demoNDIMock.json -> 'demo_ndi_mock' with its superclass entry 'demoNDI' -> 'demo_ndi'. File names stay as-is for now; renaming + updating the templates' $NDISCHEMAPATH/$NDIDOCUMENTPATH references is a separate cleanup. This clears the 11 `DID:Database:ValidationClassname` errors: buildDataset / downloadIngested / blankSession* / testConvertLinkedSessionToIngested / testDeleteIngestedSession* / testSessionList / testUnlinkSession / testIsIngestedInDataset / TestDeleteSession. (2) NDI tests + cloud-migrate doc that read base.schema_version did-matlab #141 moved schema_version to document_class. Three NDI sites still read base.schema_version and need to follow: - tests/+ndi/+unittest/+database/+internal/TestApplyReadNormalization.m (testConvertsV1StructToVDelta, testVDeltaBodyShortCircuits) - tests/+ndi/+unittest/+migrate/TestMigrateCloud.m (makeVDeltaBody) - src/ndi/+ndi/+migrate/cloud.m (doccomment only) This clears the 2 `MATLAB:nonExistentField "schema_version"` errors in TestApplyReadNormalization. Remaining open after this commit: - testCalcTuningCurve numeric failure (pre-existing, independent).
[did2 #4] aligned the demoNDI/demoNDIMock schema files with the V_delta snake_case convention applied by did2.convert.universalRenames. The write path doesn't normalize, so docs created with the legacy class name reached the validator with class_name='demoNDI' and failed against the new schemas. Rename the class everywhere it's referenced from code: the document templates (class_name, property_list_name, block keys), the document attributes registry, the example calculator, and every test fixture that builds documents via ndi.document('demoNDI') / newdocument or pokes doc_props.demoNDI.value. The underlying JSON file names (demoNDI.json, demoNDIMock.json) stay as-is — they're just the on-disk locations, not the class identity.
| mock_doc2_struct.demoNDI.value = 10; | ||
| mock_doc2 = ndi.document('demoNDIMock', 'demoNDI', mock_doc2_struct.demoNDI) + ndi_calculator_obj.session.newdocument();; | ||
| mock_doc2_struct.demo_ndi.value = 10; | ||
| mock_doc2 = ndi.document('demo_ndi_mock', 'demo_ndi', mock_doc2_struct.demo_ndi) + ndi_calculator_obj.session.newdocument();; |
| doc = E.database_search(ndi.query('demo_ndi.value','exact_number',5,'')); | ||
| if numel(doc)~=1 | ||
| error(['Found <1 or >1 document with demoNDI.value of 5; this means there is a database problem.']); | ||
| error(['Found <1 or >1 document with demo_ndi.value of 5; this means there is a database problem.']); |
| doc = E.database_search(ndi.query('','isa','demo_ndi','')); | ||
| if numel(doc)~=1 | ||
| error(['Found <1 or >1 document of type demoNDI; this means there is a database problem.']); | ||
| error(['Found <1 or >1 document of type demo_ndi; this means there is a database problem.']); |
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
3 participants
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.
Closes #776.
Summary
Flips the
NDI_DID2_NORMALIZE_ON_READgate ON unconditionally and removes the env-var plumbing. The compat layer is now demonstrably sufficient to support flipping the gate — proven by the symmetry test suite on PR #802 going green after the #801 follow-up work.Compat layer coverage (now in place on
Vnext)ndi.compat.augmentRead) for legacy field pathsndi.compat.reconcileWrite)ndi.compat.translateQueryPaths)depends_onentry-key compat viandi.documentaccessors +normalizeDependsOn, plusdaqmetadatareader.reader_classalias row,daqreader_ndrreconstruction override, and a sweep of every direct.valueread insrc/ndi/did.documentaccessors, validator field-set + subfield V_alpha↔V_delta reverse lookup,doc2sqltolerant readCleanup performed (per #779 comment 4493101768)
normalizationGateOn()and theif ~normalizationGateOn()short-circuit inndi.database.internal.applyReadNormalization. Every read body is routed throughdid2.convert.v1_to_v2.testGateOffWrapsBodyUnchanged,testGateTruthyValues, and theTestMethodSetup/TestMethodTeardownenv-var save/restore plumbing inTestApplyReadNormalization. Remaining tests renamed fromtestGateOn...to drop the gate-state prefix.augmentRead,normalizeDependsOn,reconcileWrite,translateQueryPaths) that make this default-on behaviour safe.grep -rn NDI_DID2_NORMALIZE_ON_READ src/ tests/now returns nothing.Earlier attempt
PR #800 was closed without merging because its CI surfaced two compat-layer gaps the alias-table triad alone did not cover (depends_on struct-array schema fragility, and dropped
ndi_<class>_classfields). Issue #801 tracked the audit; PR #802 + did-matlab #137/#138/#139 closed both gaps. This PR is the successor #800 was missing.Test plan
CI MATLAB runs
TestApplyReadNormalizationplus every downstream test that touchesndi.databasereads / writes / queries — including the full ndi.symmetry.makeArtifacts suite that previously surfaced the #801 issues.Generated by Claude Code