Conversation
It is no longer necessary to modify the columnType in this way
Avoid needing to collect study information in file templates
Valid values included for conditional info
…rity (14/14) CV rewritten from 19 loosely-related terms to the CDE's exact 14 values (9 reused existing codes after renaming, 5 fresh OLS lookups). Biospecimen Preservation Medium reconfirmed already exact (17/17), left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y (11/11) Dropped 13 values with no home in the real CDE (approved by user); 2 case-only fixes (Not applicable/Not reported -> Not Applicable/Not Reported). All retained codes reused from the pre-existing file, no new ontology lookups needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…6 parity File Format now backed by new modules/shared/fileFormat.csv (97 values, 97/97 exact parity: 50 reused from the old shared CV, 47 fresh OLS lookups). Dataset File Formats' overlap with the real CDE measured at only 48% - not a good-faith fit - so its legacy CDE/CRDC_CDE tag is dropped instead and its 81-value CV left as-is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Added mRNA-Seq, Bulk RNA-Seq, scRNA-Seq, scDNA-Seq via fresh OLS lookups. Kept the pre-existing RNA-Seq/DNA-Seq entries as legitimate extras rather than removing them, since they're still valid, commonly-used terms not in the CDE's list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dropped 6 inclusive/demographic values not in the CDE's permissible set (Decline to answer, Don't know, Intersex, None of these describe me, Prefer not to answer, X), per explicit user approval. All 3 retained codes reused, no new lookups. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Study_id's CDE:12960571 had no matching replacement (cdeMatch search on 4 name variants returned zero candidates - the API matches on permissible values, and Study_id is a free-form primary key with none); tag dropped, primary_key kept. DSP Data Use Codes' CDE:0002001 is likewise unresolvable; dropped, DUO values left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fill in concrete verification numbers (json_schemas diff scope, LinkML/ TTL regeneration stats, kg-pipeline test results) now that all 10 sections are implemented and verified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
update_valid_values.py + make collate + make generate-json + convert_model_to_jsonld.py + kg-pipeline's make mc2-model-linkml && make schema, reflecting all 10 sections of the CDE alignment plan (File Assay Category split, Image Assay Type, Biospecimen Embedding Medium/Preservation Method/Composition, File Format split, NGS Library Strategy, Sex, Study_id/DSP Data Use Codes). kg-pipeline's 97-test suite passes and both .ttl files parse cleanly against the regenerated schema. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ashing The live Dataset table now carries 40 fully-blank placeholder rows (every field empty except an incidental Synapse row version) that build_triples.py's mint_id() correctly refused to synthesize an id for. Skip such rows in build_triples.py's build_class_graph() and in link_scdm.py's three identifier-minting loops, logging a count, rather than hard-failing the whole build. A row missing its identifier but carrying other real data still raises, preserving the existing never-silently-drop guarantee. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…this session's model changes schema/*.ttl regenerated from the CDE-alignment model changes; data_sources.yaml and mappings/sssom/*.tsv reflect today's live extract+harmonize run (Dataset grew 1141->1181 rows, including the 40 blank rows the previous commit's fix now handles; new datacataloglicense.sssom.tsv). Published to Synapse (syn76958235, "portal-ttl-builds") via make publish-portal-kg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aset/Tool/EducationalResource models Publication Assay/Tumor Type/Tissue, Dataset Assay/Species/Tumor Type/Tissue, Tool License, and Resource License were renamed to their shared, unprefixed attribute names (Assay, Tumor Type, Tissue, Species, License) in a prior consolidation pass; this mapping file still referenced the old prefixed names. All 4 components' attribute counts already matched their current View's DependsOn list exactly - these 9 renames were the only drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… entity's stale annotation Confirmed live (2026-09-14): the Synapse Dataset entity's own native downloadType annotation disagrees with the CCKP Dataset table's curated downloadType column on 959/966 qualifying rows - the entity annotation is mostly blank or stuck at "Synapse Hosted", while the table (what the portal actually uses to decide how a dataset is presented/downloaded) says 953/977 are "Synapse Indexed". find_dataset_entity_ids() now carries the table's downloadType through and extract_datacatalog_rows() uses it verbatim instead of re-reading the unreliable entity annotation. DataCatalog.ttl now reflects 942 Synapse Indexed / 24 Synapse Hosted (re-extracted+rebuilt), matching the table. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Person stubs Grant.investigator is a scalar column but its raw values are sometimes several PI names crammed into one comma-separated string (e.g. "Amy Brock, Thomas E. Yankeelov"). Add split_person_names(): disambiguates a comma-joined list of "First MI Last" names from a single "Last, First MI" name by word count of the text before the first comma (one word -> LAST, FIRST MI, reassembled to First MI Last order; two-plus words -> a list). Documented false positive: a single-person "Last, First" name whose last name is itself multi-word (e.g. "Van't Veer, Laura") still misreads as a list - inherent to the heuristic, not hidden. EducationalResource.contributors is unaffected (already a real `|`-delimited list with no surviving commas to split on). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A multi-word segment before the first comma no longer automatically reads as a comma-joined list of names - if its first word is a known surname particle (van, von, de, mac, mc, ...), it's still one person's multi-word last name (e.g. "Van't Veer, Laura", "De La Cruz, Maria"). Matched by exact first-word comparison, not prefix, so a first name that merely starts with the same letters (e.g. "Vanessa") isn't caught by mistake. No live Grant.investigator value currently exercises this branch (checked against the current harmonized data), so scdm_links.ttl/cckp_kg_full.ttl are unchanged and don't need rebuilding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… names Two Person entries with the same first and last name are now treated as one person if their middle name/initial matches exactly (case/period- insensitive) or is missing from one of them - e.g. "Thomas Yankeelov" and "Thomas E. Yankeelov" merge into a single stub (display name upgraded to the fuller version), but "Thomas E. Yankeelov"/"Thomas J. Yankeelov" stay distinct. A missing-middle name ambiguous between 2+ already-distinguished full names is left unmerged, reusing its own stub on repeat occurrences rather than re-guessing. Every display name is also normalized to proper capitalization (proper_case_name()) before being stored - "EUN HYUN AHN" -> "Eun Hyun Ahn" - while leaving already mixed-case words (McDonald) or digit-bearing tokens (MC2) alone, since a naive str.title() would mangle either. On live data: 353 -> 336 provisional Person stubs (17 real merges), confirmed correct on the two real ALL-CAPS names that triggered this (EUN HYUN AHN, MIKALA EGEBLAD). scdm_links.ttl/cckp_kg_full.ttl regenerated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
validate_graph.py gains a --queries mode: run every queries/*.rq file (a `# name:`/`# expect:`/`# description:` header followed by a SPARQL SELECT) against one or more Turtle files, PASS if the query returns zero rows (`expect: empty` - a find-the-violations query) or at least N rows (`expect: min_count N` - a sanity floor). A lighter-weight companion to SHACL shape validation: these check graph-wide referential integrity and aggregate sanity that's awkward or impossible to express as a SHACL shape. 7 initial checks against cckp_kg_full.ttl: all 5 core classes present; investigatorRef/contributorRef edges resolve to a real sagecdm:Person; consortiumRef/institutionRef edges resolve to a real sagecdm:Program/ Organization; the DataCatalog merge lands on real cckp:Dataset subjects; every Person has exactly one display_name (regression guard for the recent name-merge work); no display_name is left ALL-CAPS (regression guard for proper_case_name()). All 7 currently pass against the live graph. Wired into `make full-kg` (after its existing SHACL check) and a new standalone `make query-checks` target. Fixture-based pytest coverage (test_query_checks.py) tests the generic engine against small test/fixtures/queries/*.rq files, the same way test_shacl_validation.py tests shacl_validate() - plus a lighter check that every real queries/*.rq file has a valid header and syntactically valid SPARQL. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Demonstration SPARQL, not pass/fail assertions (no # expect: header, so NOT picked up by validate_graph.py's --queries glob, which only reads queries/*.rq directly): tumor-type search via a resolved NCIT term, publication-dataset traceability with resolved PubMed IRIs, open-source tool discovery by operation, per-Program output rollup via the SCDM federation layer, and a DataCatalog-vs-portal-Assay curation-gap finder. Verified against the live cckp_kg_full.ttl before committing - two drafts were reworked after testing: a Tool/species/license query dropped its Tool.datasetsRef join (populated on only 2/349 Tools, too sparse to filter through), and the Program rollup was rewritten from 3 independent OPTIONALs (which effectively hung against ~330k triples) to a single join + VALUES (runs in well under a second). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bankso
force-pushed
the
cde-model-revisions
branch
from
September 14, 2026 21:23
fc98c9e to
9e7396a
Compare
queries/examples/*.rq (the 5 domain/research-question queries) deliberately have no # expect: header, so validate_graph.py's --queries mode - the only existing way to run a .rq file - errors out on them by design. There was no other way to actually execute them, which defeats the point of having them. run_query.py loads one or more Turtle files, runs a single query file (or every *.rq in a directory), and prints each query's row count and results - exploration/evaluation output, not a pass/fail assertion. Reuses validate_graph.QUERY_HEADER_RE for the optional `# name:`/`# description:` header (neither required; a multi-line description across several plain `# ...` comment lines is joined). Wired into a new `make query-examples` target. Verified against the live cckp_kg_full.ttl for both a single file and the whole examples/ directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ory API syn.tableQuery(...).asDataFrame() -> synapseclient.models.Table.query(...) (extract_cckp_tables.py, extract_datacatalog.py) - include_row_id_and_row_ version=False keeps the exact same output column shape as before. syn.get_annotations(...) -> <Model>(id=...).get(synapse_client=syn) .annotations (extract_datacatalog.py: Dataset; extract_mc2_assay_metadata.py: File, with download_file=False set on construction - the sync-wrapped File.get() downloads the file's content by default, confirmed live, so this matters: without it every annotation read would also pull real file content). syn.get(did, downloadFile=False) -> synapseclient.operations.get(did, file_options=FileOptions(download_file=False), synapse_client=syn) (extract_mc2_assay_metadata.py) - the returned model's real type replaces the old entity.concreteType string check (isinstance(entity, (Dataset, DatasetCollection))), and Dataset.items (EntityRef objects) replaces entity.properties["datasetItems"] dicts. All three read paths verified live against real Synapse entities (no deprecation warnings, output byte-identical in shape to the prior runs) - see updated test/test_datacatalog.py and test/test_mc2_assay_file_view.py for the corresponding mock updates (both now stand in for the new model-based calls directly, since neither is a plain method on a `syn` object anymore). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_full.ttl Deprecation fix: syn.getChildren(...) -> synapseclient.api.get_children(...) (an async generator, wrapped with asyncio.run() since this script is otherwise synchronous); syn.store(File(...)/Folder(...)) -> synapseclient.operations.store(...) with synapseclient.models.File/Folder (parent_id set on the entity instead of store()'s old parent= kwarg; .version_number replaces .versionNumber). Read-half (_find_child_folder_id) verified live against the real portal-ttl-builds target; the write-half (store()) is verified via mocked tests + the library's own documented usage pattern, not live-run (no test upload without being asked). New: --deploy-kg / `make deploy-kg` uploads just data/rdf/cckp_kg_full.ttl as a single File directly into its own distribution folder (syn77443315, confirmed live: publicly readable, same benefactor as the 5 portal tables) - other systems pull straight from this file, so it gets a narrower, dedicated publish path rather than being nested under publish-portal-kg's rdf/ subfolder tree-mirror. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bankso
force-pushed
the
cde-model-revisions
branch
from
September 15, 2026 18:22
8d95924 to
490120f
Compare
Bankso
added this pull request to stack #265
September 15, 2026 18:28
Document the kg-pipeline/ subsystem (previously unmentioned), remove the make qc block (that target and qc_model/mc2_qc.model.csv were removed from this branch), and drop the build-jsonld.yml CI row (removed from main as obsolete, nothing replaced it - note that no CI currently validates make all or kg-pipeline on PRs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ection Overview was missing Data Catalog from the entity-type list (added in this branch's DataCatalog integration work). Add a new "Knowledge Graph Pipeline" section introducing kg-pipeline/ and linking to its own README for setup/ commands/design rationale, and add kg-pipeline/ + json_schemas/ (already referenced in prose but missing from the tree) to Folder Structure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bankso
marked this pull request as ready for review
September 15, 2026 19:43
Compared main against cde-model-revisions directly and checked mc2-center-dcc (the DCC's portal-sync/curation tooling repo) for hardcoded coupling to the attribute names and CV values this branch changes. Groups the changes into resource model, assay model, and infrastructure/ code buckets. Two concrete, evidence-backed risks flagged for pre-merge audit: Sex's Valid Values list narrowed (114->21 chars), and Biospecimen collection-method enum values changed semantically, not just casing (json_schemas/Biospecimen.json shrank 8145->395 lines). Confirmed real coupling in mc2-center-dcc: 9+ files hardcode the exact per-entity attribute names being consolidated (centered on annotations/attribute_dictionary.py's rename maps), with two of its workflows on a monthly cron that could break unattended post-merge if DCC isn't updated first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aditya-nath-sage
approved these changes
Sep 17, 2026
aditya-nath-sage
left a comment
aditya-nath-sage
left a comment
Contributor
There was a problem hiding this comment.
Really impressive work, LGTM! It's very comprehensive with all the mappings
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.
Summary
This branch accumulates the model and knowledge-graph pipeline work since main was last updated: full CRDC CDE/CDE alignment across the model, DataCatalog and SCDM (Sage Common Data Model) integration, a new kg-pipeline knowledge-graph build system, and a curator-format migration away from schematicpy.
Note: these changes are wrapped into a single PR because they are interdependent and were developed concurrently.
[Content for review by MC2 Center team]
Model + curator migration
make convertfrom schematicpy to synapseclient's curator extension; drop schematicpy from requirements.txtCRDC CDE / CDE alignment (rounds 1-11 + final full-parity pass)
File Assay Categoryout ofAssay/DSP Dataset Assay(CRDC_CDE 12373576), resolved theImage Assay Type/GeoMx DSP Assay TypeCDE 7789196 conflict, brought Biospecimen Preservation Method/Composition/Embedding Medium, File Format, NGS Library Strategy, and Sex to exact or corrected CDE parity, and removed unresolvable legacy CDE tags (Study_id, DSP Data Use Codes)DataCatalog integration
modules/dataCatalogwith real Synapse Dataset entity annotations; extend CVs preferring reuse over new filesSCDM (Sage Common Data Model) alignment
consortium_id.csv(was pointing at a deleted file)link_scdm.py: split comma-crammedGrant.investigatorinto individual Person stubs, merge same-person name variants, proper-case display names, special-case surname particles[Prototype knowledge graph architecture]
kg-pipeline architecture
suggest_mappings.py's registry backend pluggable (add SPDX), prefix-driven rather than path-guessedqueries/*.rqsanity-query tooling (SHACL + SPARQL ASK/SELECT pass/fail checks: referential integrity, DataCatalog merge correctness, etc.), wired intomake full-kg, plus 5 domain-question example queries underqueries/examples/andscripts/run_query.pyto run thempublish_kg.py/extract_mc2_assay_metadata.pyoff deprecatedsynapseclientcalls (syn.getChildren→get_children,syn.get/syn.tableQuery→ the OOP/factory API) ahead of their removal; add--deploy-kg/make deploy-kgto publish justdata/rdf/cckp_kg_full.ttlto its own distribution folderTest plan
make all(update_valid_values -> collate -> convert -> generate-json) runs clean;mc2.model.csv,mc2.model.jsonld, and alljson_schemas/*.jsonregenerated and spot-checkedmake mc2-model-linkml && make schema && make test- 126/126 tests passfetch_cde_record()set-diff against the live caDSR recordqueries/examples/*.rqverified against the livecckp_kg_full.ttlbefore committingpublish_kg.py's migrated Synapse read paths (File.get, DatasetCollection, get_children) verified live against real Synapse entities