DID-python: class_name-first superclass resolution (schema-v2 lockstep) — review — do not merge#25
Open
audriB wants to merge 4 commits into
Open
Conversation
…step)
_get_superclass_str now reads class_name first and UNIONs the
definition-derived name in BOTH branches (top-level 'superclasses' and
'document_class.superclasses'), so schema-v2 documents with bare
{class_name} superclass objects produce the correct sorted meta.superclass
string and isa() membership. The v1 definition-only corpus is byte-for-byte
unchanged (a dict with neither key still contributes nothing).
Mirrors the merged ndi-cloud-node reference (api/src/dal/class_lineage.ts
computeClassLineage) and NDI-python ndi.document.doc_superclass, keeping
meta.superclass and both isa paths (SQL regexp + brute-force split)
consistent across all three stacks. Adds class_name + union test coverage.
Contributor
get_docs built a single 'doc_id IN (?,?,...)' with one host parameter per requested id. A cross-document query over thousands of ids on a large cloud dataset (e.g. NDI's whole-dataset relationship checks) exceeded SQLITE_MAX_VARIABLE_NUMBER (999 on older builds) → 'too many SQL variables'. Batch the id list at 900/query and accumulate; result order is restored from the doc_map, so behaviour is unchanged for small lists.
Adversarial-review remediation: hasfield interpolated field names into a LIKE pattern with '_' unescaped (single-char wildcard -> false positives); isa interpolated the class name into a regexp() pattern without escaping metacharacters (e.g. '.' over-matched dotted NDI class names). Escape both + add an ESCAPE clause; fall back to brute force on a non-numeric numeric-query operand instead of aborting the whole search. + correctness tests.
Lockstep with NDI-python: a V_epsilon flattened-diamond document (bare
{class_name} superclasses) must resolve isa() for every ancestor (both parents
+ the shared root) identically via the SQL meta.superclass path and the
brute-force field_search path. Pins the new observation-tier diamond shape; no
runtime change. 10 passed, ruff + black clean.
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
2 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.
Review — do not merge.
Schema-v2 lockstep with NDI-python (
fix/app-residuals/ PR #57) and the already-merged ndi-cloud-nodeapi/src/dal/class_lineage.ts.doc2sql._get_superclass_strnow readsclass_namefirst and unions thedefinition-derived name in both branches (top-levelsuperclassesanddocument_class.superclasses), so schema-v2 (V_delta / V_epsilon) documents whose superclasses are bare{"class_name": ...}objects produce the correct sortedmeta.superclassstring andisa()membership — across both the SQL regexp path (sqlitedb) and the brute-force split path (datastructures).{definition}superclasses (0 withclass_name), and a dict with neither key still contributes nothing — no existingmeta.superclassstring changes.computeClassLineageand NDI-pythonndi.document.doc_superclass(readclass_namefirst, never short-circuit), verified by an executable cross-stack check: all three stacks agree on a battery of dict-shaped fixtures including the discriminator{class_name:'X', definition:'.../Y.json'} -> {X, Y}.tests/test_doc2sql_superclass.py; the isa-parity regression guard stays green. Full unit suite 73 passed.