Skip to content

Isolate slots: conditional-requirement guidance, recommended/required cleanup, and isolate_meth replacement#455

Merged
pkalita-lbl merged 27 commits into
mainfrom
align-isolate-guidance
Jun 17, 2026
Merged

Isolate slots: conditional-requirement guidance, recommended/required cleanup, and isolate_meth replacement#455
pkalita-lbl merged 27 commits into
mainfrom
align-isolate-guidance

Conversation

@turbomam

@turbomam turbomam commented Jun 11, 2026

Copy link
Copy Markdown
Member

Several isolate slots were missing the "required for ..." guidance that the JGI form provides in parentheses for conditionally-required fields, which @aclum flagged in the #squad-isolates Slack thread. This adds that guidance to the comments of nuc_acid_absorb1, nuc_acid_absorb2, reference_genome, and replicate_group.

These conditions key on the JGI Sequencing Product, so the schema can't enforce them. They're guidance only. No range or constraint changes.

A follow-up will address the other part of her ask: descriptions/comments that are too verbose or inaccurate.


Update (scope grew during review). Beyond the comment guidance above, this PR now also makes enforcement changes agreed in #457 and in review:

  • recommended dropped from the two fungal-draft slots (isolate_fungal_16s_screening, isolate_its_match_unite) and from reference_genome; kept on replicate_group and the ribosomal slots.
  • collection_site_or_growth_conditions is now required on JgiIsolateTranscriptomeInterface (and no longer recommended on the genome interface).
  • isolate_meth removed in favor of the existing MIxS dna_isolate_meth / rna_isolate_meth slots (required on their interfaces); the duplicated slot listings were de-duplicated.
  • Test fixtures and example data updated accordingly, and lorem ipsum placeholders replaced with real values.

These change submission validation behavior, so the "no range or constraint changes" note above no longer holds.

Alicia asked that the "required for ..." guidance the JGI form provides in
parentheses for conditionally-required fields be reflected in the slot
comments. Adds it to nuc_acid_absorb1, nuc_acid_absorb2, reference_genome,
and replicate_group. These conditions key on the JGI Sequencing Product, so
the schema cannot enforce them; they stay as guidance.

Comments only. No range or constraint changes, the existing ranges already
match what JGI has.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 11, 2026 20:43
@github-actions

github-actions Bot commented Jun 11, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://microbiomedata.github.io/submission-schema/pr-preview/pr-455/

Built to branch gh-pages at 2026-06-17 22:42 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the NMDC submission schema’s isolate-related slot metadata to include JGI’s conditional “required for …” guidance as slot comments, providing submitters with form-aligned instructions that the schema cannot enforce as constraints.

Changes:

  • Added conditional-requirement guidance to replicate_group, nuc_acid_absorb1, nuc_acid_absorb2, and reference_genome slot comments in the base schema.
  • Regenerated derived artifacts (expanded schema YAML, JSON project output, Python datamodel header timestamps) to reflect the new comments.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/nmdc_submission_schema/schema/nmdc_submission_schema.yaml Regenerated schema YAML reflecting newly added conditional “required for …” slot comments.
src/nmdc_submission_schema/schema/nmdc_submission_schema_base.yaml Source-of-truth change adding JGI conditional-requirement guidance to the relevant isolate slots’ comments.
src/nmdc_submission_schema/datamodel/nmdc_submission_schema.py Regenerated datamodel (timestamp/header update) consistent with schema regeneration.
project/json/nmdc_submission_schema.json Regenerated JSON output including the new slot comments arrays.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@turbomam turbomam requested a review from aclum June 11, 2026 20:46
@aclum

aclum commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

I am going over the reviewing the requirements for the first two rows.
isolate_ribosomal_seq is missing information about ITS requirements " ITS sequence must be >450 nt with <2% Ns"

@aclum

aclum commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

I missed this before but First ribosomal sequence and First ribosomal sequence type are conditionally required, products that are not draft genomes (e.g. reseq) don't require this.

turbomam and others added 10 commits June 11, 2026 21:40
From Alicia's review of #455 plus a systematic diff against the JGI form
and the esplims validators:

- isolate_ribosomal_seq / isolate_ribosomal_seq_type: drop required:true.
  They are conditionally required for draft genome products only (not
  resequencing, methylation, or transcriptome), which the schema cannot
  enforce, so the condition now lives in the comments rather than an
  unconditional required:true.
- isolate_ribosomal_seq: add the fungal ITS threshold (>450 nt, <2% Ns) to
  the description; correct "all microbial and fungal genome projects" to
  "draft genome products".
- isolate_fungal_16s_screening / isolate_its_match_unite: add Fungal Light
  Draft to the required-product list (esplims req_if_fungal includes it).

Comments and requiredness only; no range or pattern changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two examples that account for dropping required:true on the ribosomal slots,
with comments recording the JGI rule LinkML cannot enforce:
- reseq-no-ribosomal: a resequencing isolate validly omits the ribosomal
  sequence (JGI requires it only for draft products).
- draft-missing-ribosomal-jgi-invalid: a draft genome missing its ribosomal
  sequence passes LinkML but is JGI-invalid; analysis_type is the same coarse
  "isolate genome sequencing" for drafts and reseq, so LinkML cannot tell them
  apart. The nmdc-server TypeScript validator is where this would be enforced.

First of a larger set documenting JGI product-conditional rules via examples.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seven more valid/ examples (each passes LinkML) with comments documenting JGI
rules LinkML cannot enforce; each would be caught by the nmdc-server TypeScript
validator:
- fungal-draft-complete: fungal draft with ITS sequence, 16S screening, and
  ITS-UNITE match, all JGI-required for fungal drafts -- JGI-valid.
- fungal-draft-missing-screening: fungal draft missing the two screening fields.
- improved-draft-missing-absorbance: missing 260/280 and 260/230.
- reseq-missing-reference-genome: resequencing without reference_genome.
- microbe-estimated-size-over-cap: 6000 Mb exceeds the 5000 Mb microbial cap
  (LinkML only bounds the outer 0-100000).
- single-cell-missing-latlon: single cell with no environmental tab for lat/lon.
- rna-expression-missing-group: transcriptome missing the replicate group name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JGI requires these slots only for certain Sequencing Products, which LinkML
cannot enforce, so they stay optional. recommended: true signals their
importance to submitters, matching how nuc_acid_absorb1/2 and cont_well are
already marked:
- isolate_ribosomal_seq, isolate_ribosomal_seq_type (draft genome products)
- isolate_fungal_16s_screening, isolate_its_match_unite (fungal drafts)
- reference_genome (resequencing, gene expression, IsoSeq, methylation)
- replicate_group (RNA expression, IsoSeq, ChIP, Bisulphite-Seq, methylation)
- collection_site_or_growth_conditions (RNA and methylation samples)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nuc_acid_concentration (ng/uL) and jgi_sample_volume (uL) carried their JGI
units only in text. Adds the structured LinkML unit (ucum_code), consistent
with the unit usage already in the schema. estimated_size (megabases) and the
absorbance ratios have no clean UCUM code, so they are left in text.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the established unit convention in the schema (e.g. the temperature
slots use descriptive_name + ucum_code): nuc_acid_concentration gets
"nanograms per microliter" and jgi_sample_volume "microliters" alongside the
UCUM codes. DataHarmonizer reads the unit/ucum_code metaslot (not the legacy
Preferred_unit/storage_units annotations), so this is the rendered form.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The isolate interface had thinner invalid-example coverage than the mature
metagenome interface. Adds single-point-of-failure counter-examples for
isolate constraints (each fails validation for exactly one reason):
- high-jgi_sample_volume (> 1000 maximum)
- bad-cont_type, bad-dnase, bad-isolate_single_colony (enum violations)
- negative-nuc_acid_concentration, negative-nuc_acid_absorb1 (below minimum)
- string-nuc_acid_absorb2 (wrong type)
- illegal-char-container_name (pattern violation)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seven of the records I had placed in valid/ pass LinkML but are not valid JGI
submissions (missing a product-conditional required field, or a value outside a
JGI category-conditional bound). They are neither valid/ (asserted to pass) nor
invalid/ (asserted to fail LinkML), so they now live in a new problem/ directory
that documents the gap between what submission-schema enforces and what JGI
requires. problem/ is not exercised by run-examples; see its README.

valid/ keeps only genuinely-valid records (reseq-no-ribosomal, fungal-draft-complete).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace absolute "LinkML cannot enforce/express/catch" wording with "we have
not found a way to express this in submission-schema with the current version
of LinkML; it may need more sophisticated rules, or nmdc-server TypeScript
validation." Cite the actual file (web/src/views/SubmissionPortal/validation.ts,
which already holds the JGI plate-well checks) and the open issues that track
the not-yet-implemented product-conditional checks (nmdc-server #2176, #2177).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each problem YAML repeated the general framing (passes-LinkML-not-JGI, the
"may need TS validation" explanation, the validation.ts path, the run-examples
note) that already lives in README.md. Reduce each header to just its specific
rule and why it slips through, plus a one-line pointer to the README and its
tracking issue (#2176 or #2177).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@turbomam

Copy link
Copy Markdown
Member Author

now includes src/data/problem with examples of submissions where the pure LinkML validation doesn't cover JGI's conditional rules

@turbomam

turbomam commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Per CONTRIBUTING.md ("This repository commits its generated files"), the regenerated build artifacts (the generated schema/nmdc_submission_schema.yaml, project/json, project/jsonschema, and datamodel/*.py) are committed alongside the source changes. Noting the choice explicitly since some recent PRs have included the generated files and others haven't.

turbomam and others added 7 commits June 12, 2026 12:31
The maintainer rationale for why each JGI product-conditional requirement is not
enforced (the requirement keys on the JGI Sequencing Product, which the schema
does not carry; or analysis_type cannot distinguish draft from resequencing) now
lives in notes: on the relevant slots (replicate_group, nuc_acid_absorb1/2,
reference_genome, isolate_ribosomal_seq, isolate_ribosomal_seq_type,
isolate_fungal_16s_screening, isolate_its_match_unite), with nmdc-server issue
references. notes is the right metaslot for editorial content: it does not render
in DataHarmonizer.

The problem/ data files keep a single-line header naming the gap (matching the
invalid/ convention) instead of a multi-line comment block. The README points to
the slot notes and records that the estimated_size cap is now proposed as a rule
in PR #456.

Co-Authored-By: Claude <noreply@anthropic.com>
Updated comments for ribosomal sequence requirements, clarifying the conditions for microbial and fungal drafts. Enhanced clarity on sequence quality and requirements.
Commit ade1830 added a two-paragraph comment on isolate_ribosomal_seq whose
second paragraph was unindented with no list marker, which broke make
schema-build and failed CI. Split it into a second list item (wording
unchanged) and regenerate the artifacts the web edit left stale.

Co-Authored-By: Claude <noreply@anthropic.com>
The problem/ examples are intentionally not run by any CI validator. State that
maintainers should shrink the set: as enforcement becomes possible (LinkML rule
or nmdc-server TypeScript), each example moves to invalid/ or is deleted.

Co-Authored-By: Claude <noreply@anthropic.com>
Resolves conflicts in regenerated files after #454 (Fix ploidy import: source OrganismSample, restore PloidyEnum range) merged. The source files (_base.yaml, config/nmdc_schema_import.yaml) auto-merged cleanly; the materialized schema and project artifacts were regenerated with make schema-build gen-project so they carry both the restored PloidyEnum range and the isolate conditional-requirement guidance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Updated description for IsolateInterface and added examples for samp_name.
Added comments to the samp_name field to provide formatting guidance.
@turbomam

turbomam commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

I am going over the reviewing the requirements for the first two rows. isolate_ribosomal_seq is missing information about ITS requirements " ITS sequence must be >450 nt with <2% Ns"

isolate_ribosomal_seq now carries the ITS threshold in both its description and its user-facing comment

ITS sequence is required for all samples for fungal drafts for strain verification. ITS sequence must be >450 nt with <2% Ns

Also keeps the other product thresholds (>1300 nt for microbial drafts, >220 bp with <10% Ns for single cells).

Nothing's checking the proportion of N bases or conditional lengths. I think we will have to do that in

@turbomam

Copy link
Copy Markdown
Member Author

I missed this before but First ribosomal sequence and First ribosomal sequence type are conditionally required, products that are not draft genomes (e.g. reseq) don't require this.

Both isolate_ribosomal_seq and isolate_ribosomal_seq_type now note that JGI requires the ribosomal sequence for draft genome products but not for resequencing, methylation, or transcriptome products.

We can't express that as a LinkML rule because the requirement keys on the JGI Sequencing Product, which the schema doesn't carry as a slot, so it stays as guidance in the slot comments for now

see also

Removed 'isolate_meth' field and related references from the schema. use existing rna and dna specific slots instead
Comment thread src/nmdc_submission_schema/schema/nmdc_submission_schema.yaml Outdated
Comment thread src/nmdc_submission_schema/schema/nmdc_submission_schema.yaml Outdated
aclum and others added 2 commits June 12, 2026 12:22
re-use existing rna and dna isolation slots
Completes 705dc55
(Remove isolate_meth field from submission schema), which dropped the slot from the source
but left the generated artifacts stale (they still contained isolate_meth) and left the slot
in 34 example files and 2 test template instances.

Per the agreed design (use the existing dna_isolate_meth and rna_isolate_meth slots, which are
required on the genome and transcriptome isolate interfaces): substituted isolate_meth with
dna_isolate_meth in the DNA/genome examples and rna_isolate_meth in the RNA/transcriptome
examples, updated the two affected template instances, and regenerated the schema and project
artifacts from source so isolate_meth is gone everywhere. make test passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add examples and description for genome size estimation.
Comment thread config/nmdc_schema_import.yaml
… on transcriptome; fix genome-size example form

Per @aclum on #457: drop recommended from reference_genome and the two fungal slots; keep it on replicate_group and the ribosomal slots; make collection_site_or_growth_conditions required on JgiIsolateTranscriptomeInterface and not recommended on the genome interface.

Also fixes the genome-size estimate example in config/nmdc_schema_import.yaml to the value: mapping form (a bare scalar broke the LinkML build). Added collection_site to the transcriptome valid example and template, plus a transcriptome-missing-collection_site counter-example. make test passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

config/nmdc_schema_import.yaml:1222

  • dna_isolate_meth is now injected into JgiIsolateGenomeInterface via import destinations, but it is also explicitly listed in JgiIsolateGenomeInterface.slots in the base schema. In the generated schema, this results in dna_isolate_meth appearing twice in the class slots list (and duplicate validation code in the generated datamodel). Ensure the slot is introduced exactly once (either via import destinations or via the base class slot list) and regenerate artifacts.
          slot_group: mixs_core_section

  - slot: dna_isolate_meth
    source: Biosample
    destinations:
      - JgiMgInterface
      - JgiMgLrInterface
      - JgiIsolateGenomeInterface
    modifications:
      - replace:
          rank: 16
          slot_group: jgi_metagenomics_section
          required: true
          recommended: false

config/nmdc_schema_import.yaml:3832

  • rna_isolate_meth is now injected into JgiIsolateTranscriptomeInterface via import destinations, but it is also explicitly listed in JgiIsolateTranscriptomeInterface.slots in the base schema. In the generated schema, this results in rna_isolate_meth appearing twice in the class slots list (and duplicate validation code in the generated datamodel). Ensure the slot is introduced exactly once (either via import destinations or via the base class slot list) and regenerate artifacts.
  - slot: rna_isolate_meth
    source: Biosample
    destinations:
      - JgiMtInterface
      - JgiIsolateTranscriptomeInterface
    modifications:
      - replace:
          rank: 16
          slot_group: jgi_metatranscriptomics_section
          required: true
          recommended: false

Comment thread src/nmdc_submission_schema/datamodel/nmdc_submission_schema.py Outdated
Comment thread src/nmdc_submission_schema/datamodel/nmdc_submission_schema.py Outdated
Comment thread src/nmdc_submission_schema/schema/nmdc_submission_schema.yaml
Comment thread src/nmdc_submission_schema/schema/nmdc_submission_schema.yaml
Comment thread src/nmdc_submission_schema/schema/nmdc_submission_schema_base.yaml
turbomam and others added 2 commits June 12, 2026 18:58
…iew)

dna_isolate_meth and rna_isolate_meth are imported slots routed onto the JGI interfaces via
config/nmdc_schema_import.yaml destinations, but they were also listed directly in each
interface's slots: in _base.yaml. That double-listed them, cascading into duplicate
required/type-check code in the generated datamodel. Removed the redundant direct listings
(slot_usage and import routing unchanged) so each interface carries the slot once.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The JGI interface template instances used 'lorem ipsum' for dna_isolate_meth,
rna_isolate_meth, and env_broad_scale. Replaced with real values: CTAB chloroform
extraction (DNA) and TRIzol RNA extraction (RNA), both standard methods already used in
the curated isolate examples, and 'terrestrial biome [ENVO:00000446]' (OLS-verified) for
env_broad_scale. Synthetic contact/PI names left as-is.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@turbomam turbomam changed the title Add JGI conditional-requirement guidance to isolate slot comments Isolate slots: conditional-requirement guidance, recommended/required cleanup, and isolate_meth replacement Jun 15, 2026
@aclum

aclum commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

@pkalita-lbl is there a way to make the docs preview work? The link above gets me a 404 (https://microbiomedata.github.io/submission-schema/pr-preview/pr-455/)

@pkalita-lbl

Copy link
Copy Markdown
Collaborator

is there a way to make the docs preview work?

@aclum Not easily. The workflow that builds the docs site to include the DataHarmonizer playground isn't readily compatible with the way the PR preview actions expects the workflow to be setup. We might be able to make it work, but I don't know what that would look like off the top of my head.

@aclum

aclum commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

okay, no problem.

@aclum aclum requested review from mslarae13 and pkalita-lbl June 17, 2026 22:06
@pkalita-lbl pkalita-lbl merged commit 6f67867 into main Jun 17, 2026
2 checks passed
@pkalita-lbl pkalita-lbl deleted the align-isolate-guidance branch June 17, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants