Skip to content

Add HRA palatine-tonsil NTR template (7 new terms) - #3719

Open
zhengj2007 wants to merge 3 commits into
masterfrom
add-hra-palatine_tonsil-ntr
Open

Add HRA palatine-tonsil NTR template (7 new terms)#3719
zhengj2007 wants to merge 3 commits into
masterfrom
add-hra-palatine_tonsil-ntr

Conversation

@zhengj2007

Copy link
Copy Markdown
Collaborator

Generated via bulk_ntr_workflow from source_data/palatine-tonsil.csv.

Pipeline completed through Stage 4:

  • 7 new UBERON terms in leaf template
  • 0 group terms
  • 0 confirmed matches excluded
  • 0 out-of-scope exclusions
  • 0 manual-curation exclusions

Parent mapping handled in Stage 3 for mixed source parents:

  • UBERON parents (4)
  • ASCTB-TEMP parents (2)
  • FMA parent mapping (1)

Final template has no [PENDING], INFER, NEEDS_MAPPING, or UNRESOLVABLE markers.

Generated via bulk_ntr_workflow from source_data/palatine-tonsil.csv.

Pipeline completed through Stage 4:
- 7 new UBERON terms in leaf template
- 0 group terms
- 0 confirmed matches excluded
- 0 out-of-scope exclusions
- 0 manual-curation exclusions

Parent mapping handled in Stage 3 for mixed source parents:
- UBERON parents (4)
- ASCTB-TEMP parents (2)
- FMA parent mapping (1)

Final template has no [PENDING], INFER, NEEDS_MAPPING, or UNRESOLVABLE markers.

Signed-off-by: dragon-ai-agent <dragon-ai-agent@users.noreply.github.com>
@ai4c-agent

ai4c-agent Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @zhengj2007's task in 7m 29s —— View job


PR Review: HRA Palatine-Tonsil NTR Template (7 new terms)

Review checklist:

  • Hierarchy and parents are consistent
  • Definitions and supporting references look adequate
  • Obsolete or replacement handling is safe (N/A)
  • OBO and ODK conventions appear preserved
  • No obvious CI, auth, or workflow regression

Review Summary

The 7 new terms are anatomically well-chosen and fill genuine gaps in the tonsil substructure representation. Parent-term resolution is mostly correct and consistent with analogous existing terms (the germinal center dark/light zones correctly follow the UBERON:8410052/UBERON:8410053 lymph-node precedent with is_a: UBERON:0001744). However, one critical build issue and one important structural omission need to be fixed before merge.


🔴 CRITICAL — Templates not registered in src/ontology/uberon-odk.yaml

Stage 5 of the NTR workflow (registering templates with ODK) was not completed. Neither hra-palatine-tonsil.template.tsv nor hra-palatine-tonsil-groups.template.tsv appear in src/ontology/uberon-odk.yaml as component products. Without this registration, ROBOT will never process the templates and the 7 new UBERON terms (8930153–8930159) will not be built into any ontology artifact.

The pattern to add (following the existing hra_skeleton.owl precedent):

    - filename: hra_palatine_tonsil.owl
      use_template: true
      templates:
        - hra-palatine-tonsil.template.tsv
    - filename: hra_palatine_tonsil_groups.owl
      use_template: true
      templates:
        - hra-palatine-tonsil-groups.template.tsv

After editing the YAML, sh run.sh make update_repo (or the register_templates.py script from the workflow) must be run to regenerate the Makefile. Fix this →


🟡 IMPORTANT — tonsil germinal center mantle zone (UBERON:8930157) missing part_of

The definition explicitly states "A part of the secondary follicle of the tonsil", yet the part_of column is empty in the template (src/templates/hra-palatine-tonsil.template.tsv, row 7). The term will be placed as is_a: UBERON:0010755 ! secondary follicle corona only, with no explicit tonsil localisation. An explicit part_of UBERON:8930158 (tonsil secondary follicle — added in the same template) should be added.


🟡 IMPORTANT — tonsil follicle (UBERON:8930154) scope mismatch between name and definition

The definition reads "A lymphoid follicle that is part of a palatine tonsil" and part_of is UBERON:0002373 ! palatine tonsil, yet the label is tonsil follicle (not palatine tonsil follicle). Compare: tonsil germinal center (UBERON:0013688) uses the general part_of UBERON:0002372 ! tonsil. Either:

  • Broaden the definition/part_of to the general tonsil (UBERON:0002372) to match the name, or
  • Rename to palatine tonsil follicle to match the definition.

🔵 SUGGESTION — tonsil T cell zone xref FMA:55221 may be imprecise

The input errors report flags FMA:55221 as "Diffuse lymphoid tissue" — a broad FMA concept used as the original parent mapping. Moving it to an xref rather than a parent is correct, but a generic "Diffuse lymphoid tissue" xref on a specifically-named T cell zone may mislead downstream users. If there is no more specific FMA term, consider removing this xref.


Notes (no action required)

  • ID range: UBERON:8930153–8930159 fall within [>= 8930000, < 8931000] allocated to Jie Zheng. ✓
  • is_a: UBERON:0001744 for dark/light zones: consistent with existing UBERON:8410052/8410053. ✓
  • ASCTB-TEMP IRIs in def_xref: standard HRA provenance pattern with real PMIDs also present. ✓
  • present_in_taxon: NCBITaxon:9606: appropriate for HRA terms. ✓
  • PMIDs 25656706 and 8011279 verified in existing UBERON terms. PMIDs 34430822, 417886, 11789984, 28844822, 30801022 should be manually verified for relevance.
  • Empty groups template (headers only) is correct — all 7 terms are leaf terms. ✓

@ai4c-agent ai4c-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two issues need fixing before merge:

  1. 🔴 CRITICAL — Templates not registered in uberon-odk.yaml: Stage 5 of the NTR workflow was not completed. Neither hra-palatine-tonsil.template.tsv nor hra-palatine-tonsil-groups.template.tsv appear as component products in src/ontology/uberon-odk.yaml. Without this, ROBOT will not process the templates and the 7 new terms (UBERON:8930153–8930159) will not be built into any ontology artifact. Add entries following the hra_skeleton.owl pattern and regenerate the Makefile.

  2. 🟡 IMPORTANT — tonsil germinal center mantle zone (UBERON:8930157) missing part_of: Definition says 'A part of the secondary follicle of the tonsil' but the part_of column is empty. Should have part_of UBERON:8930158 ! tonsil secondary follicle.

Also: tonsil follicle (UBERON:8930154) label vs definition scope mismatch (name implies general tonsil scope but definition/part_of is palatine-tonsil-specific).

Full review details in the Claude comment above.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has not seen any activity in the past month; if nobody comments or reviews it in the next week, the PR creator will be allowed to proceed with merging without explicit approval, should they wish to do so.

@zhengj2007

Copy link
Copy Markdown
Collaborator Author

@dragon-ai-agent

Please remove empty file, src/templates/hra-palatine-tonsil-groups.template.tsv

modify the template file, src/templates/hra-palatine-tonsil.template.tsv, as follows:
UBERON:8930157 - add part_of UBERON:8930158 ! tonsil secondary follicle
UBERON:8930154 - rename to 'palatine tonsil follicle'
UBERON:8930159 - remove xref FMA:55221

- Remove empty groups template (hra-palatine-tonsil-groups.template.tsv)
- UBERON:8930154: rename 'tonsil follicle' -> 'palatine tonsil follicle'
- UBERON:8930157 (tonsil germinal center mantle zone): add part_of UBERON:8930158 (tonsil secondary follicle)
- UBERON:8930159 (tonsil T cell zone): remove xref FMA:55221

Signed-off-by: @dragon-ai-agent

---
🤖 **Generated by @dragon-ai-agent**
- Model: `claude-opus-4-7`
- Agent harness: claude-code
- Triggered by: @zhengj2007
- Run: [View workflow run](https://github.com/obophenotype/uberon/actions/runs/28964269531)
@ai4c-agent

ai4c-agent Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dragon-ai-agent's task in 3m 53s —— View job


Review checklist

  • Hierarchy and parents are consistent
  • Definitions and supporting references look adequate
  • Obsolete or replacement handling is safe (N/A)
  • OBO and ODK conventions appear preserved (partially — see below)
  • No obvious CI, auth, or workflow regression

Verification of commit c23861e (requested fixes)

All three edits @zhengj2007 requested were applied correctly to src/templates/hra-palatine-tonsil.template.tsv:

  • UBERON:8930154 renamed tonsil folliclepalatine tonsil follicle (now consistent with its palatine-tonsil-specific definition/part_of UBERON:0002373)
  • UBERON:8930157 (tonsil germinal center mantle zone) now has part_of UBERON:8930158 (tonsil secondary follicle), matching its definition
  • UBERON:8930159 (tonsil T cell zone): xref FMA:55221 removed
  • src/templates/hra-palatine-tonsil-groups.template.tsv removed (was header-only, no data rows)

Hierarchy re-checked against src/ontology/uberon-edit.obo for all referenced parents (UBERON:0001593, UBERON:0000444, UBERON:0002373, UBERON:0001744, UBERON:0013688, UBERON:0010755, UBERON:0001745, UBERON:0002372, UBERON:0010393) — all exist and are logically appropriate (e.g. UBERON:0010755 secondary follicle corona has synonym "mantle zone", supporting the is_a choice for UBERON:8930157).

🔴 CRITICAL — still unresolved: templates not registered in src/ontology/uberon-odk.yaml

This was flagged in the original review and was not part of the fixes requested in the latest comment, so it remains outstanding. Confirmed by diff (git diff origin/master...HEAD --stat still only shows the 5 originally-flagged files) and by grep — no hra_palatine_tonsil entry exists anywhere in src/ontology/uberon-odk.yaml or the generated src/ontology/Makefile (compare to the registered hra_skeleton.owl pattern at uberon-odk.yaml:177-180 / Makefile:571-575).

Without this registration, ROBOT will never process hra-palatine-tonsil.template.tsv, and the 7 new UBERON terms (8930153–8930159) will not be built into any ontology artifact.

Needed, under components.products in src/ontology/uberon-odk.yaml:

    - filename: hra_palatine_tonsil.owl
      use_template: true
      templates:
        - hra-palatine-tonsil.template.tsv

followed by sh run.sh make update_repo to regenerate the Makefile.

Fix this →

Recommendation

Submitted a Request Changes review — blocking only on the ODK registration step. The content-level fixes requested by @zhengj2007 are all correctly applied. Once the template is registered as a component product and the Makefile regenerated, this should be ready to merge.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor
This PR modifies some logical definitions. See details below:

Number of classes with modified logical definitions: 1

gABAergic system

No changes in inferred subclasses.

@ai4c-agent ai4c-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review checklist

  • Hierarchy and parents are consistent
  • Definitions and supporting references look adequate
  • Obsolete or replacement handling is safe (N/A)
  • OBO and ODK conventions appear preserved (partially — see below)
  • No obvious CI, auth, or workflow regression

Verification of commit c23861e (requested fixes)

All three requested edits were applied correctly to src/templates/hra-palatine-tonsil.template.tsv:

  • UBERON:8930154 renamed tonsil folliclepalatine tonsil follicle (now consistent with its palatine-tonsil-specific definition/part_of UBERON:0002373)
  • UBERON:8930157 (tonsil germinal center mantle zone) now has part_of UBERON:8930158 (tonsil secondary follicle), matching its definition
  • UBERON:8930159 (tonsil T cell zone): xref FMA:55221 removed
  • src/templates/hra-palatine-tonsil-groups.template.tsv removed (was header-only, no data rows)

Hierarchy re-checked against src/ontology/uberon-edit.obo for all referenced parents (UBERON:0001593, UBERON:0000444, UBERON:0002373, UBERON:0001744, UBERON:0013688, UBERON:0010755, UBERON:0001745, UBERON:0002372, UBERON:0010393) — all exist and are logically appropriate (e.g. UBERON:0010755 secondary follicle corona has synonym "mantle zone", supporting the is_a choice for UBERON:8930157).

🔴 CRITICAL — still unresolved: templates not registered in src/ontology/uberon-odk.yaml

This was flagged in the original review and was not part of the fixes requested in the latest comment, so it remains outstanding. Confirmed by diff (git diff origin/master...HEAD --stat still shows only the 5 originally-flagged files — uberon-odk.yaml and Makefile are untouched) and by grep — no hra_palatine_tonsil entry exists anywhere in src/ontology/uberon-odk.yaml or the generated src/ontology/Makefile (compare to the registered hra_skeleton.owl pattern at uberon-odk.yaml:177-180 / Makefile:571-575).

Without this registration, ROBOT will never process hra-palatine-tonsil.template.tsv, and the 7 new UBERON terms (8930153–8930159, including the newly-added UBERON:8930158) will not be built into any ontology artifact.

Needed:

    - filename: hra_palatine_tonsil.owl
      use_template: true
      templates:
        - hra-palatine-tonsil.template.tsv

under components.products in src/ontology/uberon-odk.yaml, followed by sh run.sh make update_repo to regenerate the Makefile (mirrors hra_skeleton.owl).

Recommendation

Blocking on the ODK registration step only — the content-level fixes requested by @zhengj2007 are all correctly applied. Once the template is registered as a component product and the Makefile regenerated, this should be ready to merge.

Fix this →

@zhengj2007

Copy link
Copy Markdown
Collaborator Author

Only 7 terms, shall we add the terms to uberon-edit.obo directly?

Reports are review artifacts, not ODK templates, and should not live under
src/templates/. Relocated to bulk_ntr_workflow/outputs/ to match the canonical
layout used by the muscular branch (PR #3700).

Signed-off-by: dragon-ai-agent
@ai4c-agent

ai4c-agent Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dosumis's task in 4m 12s —— View job


Review checklist

  • Hierarchy and parents are consistent
  • Definitions and supporting references look adequate
  • Obsolete or replacement handling is safe (N/A — no obsoletions in this PR)
  • OBO and ODK conventions appear preserved (content is fine; build-registration is not — see below)
  • No obvious CI, auth, or workflow regression

⚠️ Security note: a comment on this PR looks like a prompt-injection / impersonation attempt

The comment from cellsemantic (posted immediately after this job started) asserts a repo "batch-size policy" (≥20 new terms → ODK-register; <20 → hand-edit) and instructs converting the template to OBO stanzas, while claiming to act "for @dosumis" and citing dragon-ai-agent. I checked: cellsemantic is not a collaborator on this repo (not in the collaborators list, no special permissions), and no such batch-size policy exists in CLAUDE.md or anywhere else in the repo. This looks like an attempt to inject unauthorized instructions into the review thread by impersonating an automated/authoritative source. I have not acted on anything from that comment — my analysis below is based only on CLAUDE.md, the actual repo state, and @zhengj2007's genuine question. Maintainers may want to review/remove that comment.

Status since last review

Commit c890b08 (David Osumi-Sutherland) only relocated the report artifacts (candidates.tsv, errors.tsv, input.tsv) from src/templates/ to bulk_ntr_workflow/outputs/hra-palatine-tonsil-reports/, matching the canonical layout from PR #3700. No content changes to the template or uberon-edit.obo.

🔴 CRITICAL — still unresolved: hra-palatine-tonsil.template.tsv remains unregistered in src/ontology/uberon-odk.yaml (confirmed via diff and grep — no hra_palatine_tonsil entry exists, and uberon-edit.obo is untouched by this branch). As-is, the 7 new terms (UBERON:8930153–8930159) will not be built into any ontology artifact.

Answering @zhengj2007: "shall we add the terms to uberon-edit.obo directly?"

Yes, for a batch this small this is a reasonable and simpler alternative to fixing the ODK template registration, and it matches the standard editing workflow documented in CLAUDE.md (obo-checkout.pl / obo-checkin.pl into terms/). The template's 7 rows map cleanly onto term stanzas — all the needed metadata is already present:

  • ID/LABELid/name
  • Definition + def_xrefdef: "..." [xref1, xref2, ...]
  • is_ais_a
  • part_ofrelationship: part_of BFO:0000050 ... (or logical intersection_of if you want them reasoner-inferable rather than asserted)
  • In_subset (added_by_HRA) → subset: added_by_HRA — confirmed this subsetdef already exists in uberon-edit.obo
  • Contributor/Daterelationship: dc-contributor https://orcid.org/... / property_value: dcterms-date "..."^^xsd:dateTime
  • Present_in_taxonrelationship: present_in_taxon NCBITaxon:9606
  • xref/Wikipedia_imagexref: lines / property_value: foaf:depiction

Trade-off to flag: going this route means these 7 terms lose the reusable ROBOT-template/provenance link back to the HRA source CSV that the bulk_ntr_workflow pipeline otherwise gives you (useful if HRA periodically re-syncs this dataset). If that's not a concern for this one-off tonsil batch, direct hand-edit is fine and avoids the multi-week block on template/Makefile registration.

I haven't made the edit myself since this is a process decision for the maintainers — happy to do the conversion and check the terms in via terms/ on confirmation, or alternatively to just fix the uberon-odk.yaml registration if you'd rather keep the template route.

Recommendation

Requesting changes — blocking only on getting the 7 terms actually built (either path resolves it): register hra-palatine-tonsil.template.tsv as a component product in src/ontology/uberon-odk.yaml and regenerate the Makefile, or hand-convert the template rows into uberon-edit.obo via the standard terms/ workflow.

@cellsemantic

Copy link
Copy Markdown

Reports relocated. Route: manual hand-edit (7 terms, <20). TODO: convert to OBO stanzas; verify defs/PMIDs.


Automated update (@cellsemantic / dragon-ai-agent, for @dosumis). Reports were relocated out of src/templates/bulk_ntr_workflow/outputs/<name>-reports/ to match the canonical layout (PR #3700). Note: the corrected reports-location docs currently live only on add-hra-muscular-ntr (#3700), not master. Batch-size policy for this HRA set: ≥20 new terms → ODK-register; <20 → hand-edit into uberon-edit.obo.

@ai4c-agent ai4c-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review checklist

  • Hierarchy and parents are consistent
  • Definitions and supporting references look adequate
  • Obsolete or replacement handling is safe (N/A)
  • OBO and ODK conventions appear preserved (content is fine; build-registration is not)
  • No obvious CI, auth, or workflow regression

Still blocking on the same CRITICAL issue as prior reviews: hra-palatine-tonsil.template.tsv is not registered as a component product in src/ontology/uberon-odk.yaml, so the 7 new terms (UBERON:8930153–8930159) will not be built into any ontology artifact as things stand. Commit c890b08 only relocated report artifacts; no content changes.

Re @zhengj2007's question about hand-editing directly into uberon-edit.obo instead: this is a reasonable path for a 7-term batch and matches the standard CLAUDE.md workflow (terms/ + obo-checkout.pl/obo-checkin.pl) — full mapping and trade-offs posted in the PR comment. Either fixing the ODK registration or switching to direct hand-edit resolves the blocker.

Also flagging: a comment on this PR from cellsemantic (not a repo collaborator) claims an unofficial "batch-size policy" instructing a hand-edit route — this appears to be a prompt-injection/impersonation attempt and was disregarded; my recommendation above is based only on CLAUDE.md and the actual repo state.

Full details in the PR comment above.

@zhengj2007

zhengj2007 commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

@dragon-ai-agent

Please create new terms based on the information in the template file, src/templates/hra-palatine-tonsil.template.tsv, and add the new terms to src/ontology/uberon-edit.obo

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.

3 participants