Repository navigation
Add HRA palatine-tonsil NTR template (7 new terms) - #3719
zhengj2007 wants to merge 9 commits into
Conversation
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>
|
Claude finished @zhengj2007's task in 7m 29s —— View job PR Review: HRA Palatine-Tonsil NTR Template (7 new terms)Review checklist:
Review SummaryThe 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 🔴 CRITICAL — Templates not registered in
|
![ai4c-agent[bot]](https://avatars.githubusercontent.com/in/2242960?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two issues need fixing before merge:
-
🔴 CRITICAL — Templates not registered in
uberon-odk.yaml: Stage 5 of the NTR workflow was not completed. Neitherhra-palatine-tonsil.template.tsvnorhra-palatine-tonsil-groups.template.tsvappear as component products insrc/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 thehra_skeleton.owlpattern and regenerate the Makefile. -
🟡 IMPORTANT —
tonsil germinal center mantle zone(UBERON:8930157) missingpart_of: Definition says 'A part of the secondary follicle of the tonsil' but the part_of column is empty. Should havepart_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.
|
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. |
|
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: |
|
🤖 Working on it... Follow along: View workflow run |
- 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)
|
Applied the requested changes in commit c23861e:
CRLF line endings and 13-column TSV structure preserved. 🤖 Generated by @dragon-ai-agent
|
|
Claude finished @dragon-ai-agent's task in 3m 53s —— View job Review checklist
Verification of commit c23861e (requested fixes)All three edits @zhengj2007 requested were applied correctly to
Hierarchy re-checked against 🔴 CRITICAL — still unresolved: templates not registered in
|
This PR modifies some logical definitions. See details below:Number of classes with modified logical definitions: 4 tonsil T cell zoneNo changes in inferred subclasses. tonsil germinal center mantle zoneNo changes in inferred subclasses. tonsil secondary follicleNo changes in inferred subclasses. palatine tonsil follicleNo changes in inferred subclasses. |
There was a problem hiding this comment.
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:8930154renamedtonsil follicle→palatine tonsil follicle(now consistent with its palatine-tonsil-specific definition/part_of UBERON:0002373) - ✅
UBERON:8930157(tonsil germinal center mantle zone) now haspart_of UBERON:8930158(tonsil secondary follicle), matching its definition - ✅
UBERON:8930159(tonsil T cell zone):xref FMA:55221removed - ✅
src/templates/hra-palatine-tonsil-groups.template.tsvremoved (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.tsvunder 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.
|
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
|
Claude finished @dosumis's task in 4m 12s —— View job Review checklist
|
|
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 |
There was a problem hiding this comment.
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.
|
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 |
|
Claude finished @zhengj2007's task in 10m 52s —— View job Task: hand-convert the 7 template terms into
|
![ai4c-reviewer[bot]](https://avatars.githubusercontent.com/in/4388239?s=60&v=4){kind=small}
There was a problem hiding this comment.
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-level; build-registration is not)
- No obvious CI, auth, or workflow regression
The only new commit since the last review is a merge of master into this branch, bringing in unrelated upstream changes but no content changes to this PR's own files (confirmed via git diff origin/master...HEAD --stat: still the same 5 files, +30/-0).
All previously-requested content fixes remain correctly applied to src/templates/hra-palatine-tonsil.template.tsv (palatine tonsil follicle rename, mantle-zone part_of, FMA:55221 xref removal, groups template removal).
🔴 CRITICAL — still unresolved: verified directly that none of UBERON:8930153–8930159 exist in src/ontology/uberon-edit.obo, and no hra_palatine_tonsil entry exists in src/ontology/uberon-odk.yaml or the generated Makefile. As things stand this PR adds only a template with no corresponding build product, so the 7 new terms will not be built into the released ontology.
The PR author has since asked (2026-07-19) to instead hand-convert the template rows directly into uberon-edit.obo via the standard terms/ workflow — that is a reasonable resolution but has not yet been actioned on this branch. Either registering the template in uberon-odk.yaml or hand-editing the terms into uberon-edit.obo would clear this blocker.
Full details: #3719 (comment)
|
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. |
Hand-convert the 7 template rows into OBO stanzas (<20-term policy, as requested by @zhengj2007 on 2026-07-19) and remove the unregistered template, which was never built so the terms were not in the ontology. Modelling follows the existing lymph node analogues: - mantle zone: intersection_of secondary follicle corona + part_of tonsil, part_of tonsil secondary follicle, surrounds tonsil germinal center (cf. UBERON:0010420) - tonsil secondary follicle, tonsil T cell zone, palatine tonsil follicle: genus-differentia logical definitions (cf. UBERON:0010753, UBERON:0010417) - dark/light zone: is_a lymphoid tissue, part_of tonsil germinal center (cf. UBERON:8410052/8410053) - peritonsillar plexus of veins: located_in (not part_of) tonsillar fossa, since the fossa is an immaterial anatomical space (part_of would be unsatisfiable under material/immaterial disjointness). Also: ASCTB-TEMP IRIs moved from def xrefs to CCF: xrefs; DOI URLs as DOI: CURIEs; dropped Wikipedia xrefs pointing at different structures and a follicular-lymphoma (pathology) depiction image. Signed: @ai4c-agent Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Claude finished @dosumis's task in 4m 9s —— View job Review checklist
What changed since the last reviewCommit
🟡 IMPORTANT — mantle zone logical definition still doesn't fully mirror its text definitionLeft inline on 🔵 SUGGESTION — missing
|
| subset: added_by_HRA | ||
| xref: CCF:ASCTB-TEMP_tonsil-germinal-center-mantle-zone | ||
| intersection_of: UBERON:0010755 ! secondary follicle corona | ||
| intersection_of: part_of UBERON:0002372 ! tonsil |
There was a problem hiding this comment.
🟡 IMPORTANT — logical definition still doesn't mirror the text definition for tonsil germinal center mantle zone (UBERON:8930157).
The def says "A part of the secondary follicle of the tonsil...", and an asserted relationship: part_of UBERON:8930158 ! tonsil secondary follicle was correctly added in commit c23861e per @zhengj2007's request. But the genus-differentia intersection_of (the logical definition CLAUDE.md says should mirror the text def) still uses the broader part_of UBERON:0002372 ! tonsil rather than the more specific UBERON:8930158. The original source data (bulk_ntr_workflow/source_data/palatine-tonsil.csv) also lists tonsil-secondary-follicle as this term's intended parent, not the general tonsil.
Suggested fix — replace line 225346:
| intersection_of: part_of UBERON:0002372 ! tonsil | |
| intersection_of: part_of UBERON:8930158 ! tonsil secondary follicle |
Since UBERON:8930158 is itself part_of tonsil, this doesn't change the asserted facts, just tightens the reasoner-facing logical definition to match the text def and removes the now-redundant explicit relationship: part_of UBERON:8930158 line above it (or keep both, but the intersection_of is the one that should encode the specific parent).
| subset: added_by_HRA | ||
| xref: CCF:ASCTB-TEMP_peritonsillar-plexus-of-veins | ||
| is_a: UBERON:0001593 ! venous plexus | ||
| relationship: dc-contributor https://orcid.org/0000-0002-2999-0103 |
There was a problem hiding this comment.
🔵 SUGGESTION — dc-contributor lines are missing the ! <NAME> comment.
All 7 new stanzas use relationship: dc-contributor https://orcid.org/0000-0002-2999-0103 with no trailing ! <Name> (same pattern at lines 225294, 225308, 225320, 225333, 225347, 225362, 225375). CLAUDE.md specifies the format relationship: dc-contributor https://orcid.org/<ORCID> ! <NAME>, and existing terms in the file consistently include the name comment (e.g. UBERON:8410052 ! Paola Roncaglia). Not build-breaking, but worth adding for readability/provenance — presumably this resolves to Jie Zheng (@zhengj2007), the term requester.
There was a problem hiding this comment.
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
- No obvious CI, auth, or workflow regression
The 🔴 CRITICAL ODK-registration blocker from prior rounds is resolved: the 7 terms (UBERON:8930153–8930159) are now hand-converted stanzas directly in src/ontology/uberon-edit.obo and the unregistered template was removed. Parents, typedefs, ID-range allocation, and prior content fixes (palatine tonsil follicle rename, FMA:55221 removal) all check out.
One issue remains, see inline comment on src/ontology/uberon-edit.obo:225346:
🟡 IMPORTANT — tonsil germinal center mantle zone (UBERON:8930157): the def text says "A part of the secondary follicle of the tonsil", and an asserted part_of UBERON:8930158 was added, but the intersection_of (logical definition) still uses the broader part_of UBERON:0002372 ! tonsil instead of UBERON:8930158 ! tonsil secondary follicle, so the logical definition doesn't fully mirror the text definition as CLAUDE.md requires. Suggested one-line fix included inline.
🔵 SUGGESTION (non-blocking): all 7 new dc-contributor lines are missing the ! <NAME> comment CLAUDE.md's template calls for — see inline comment on line 225294.
Full details in the PR comment thread.
…nsil-ntr # Conflicts: # src/ontology/uberon-edit.obo
|
Claude finished @dosumis's task in 1m 17s —— View job Review checklist
StatusNo new content commits since the last review round —
🟡 IMPORTANT — still open:
|
There was a problem hiding this comment.
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
- No obvious CI, auth, or workflow regression
No content changes since the last review round (only a merge-from-master commit). The 🔴 CRITICAL ODK-registration blocker from earlier rounds remains resolved (7 terms hand-converted into uberon-edit.obo, template removed, no uberon-odk.yaml registration needed).
One issue remains open, unaddressed since it was first flagged:
🟡 IMPORTANT — tonsil germinal center mantle zone (UBERON:8930157): the text def says "A part of the secondary follicle of the tonsil," and an asserted relationship: part_of UBERON:8930158 (tonsil secondary follicle) was added, but the intersection_of (logical definition) still uses the broader part_of UBERON:0002372 ! tonsil instead of UBERON:8930158, so the logical definition doesn't mirror the text definition as CLAUDE.md requires. Suggested fix: change intersection_of: part_of UBERON:0002372 ! tonsil to intersection_of: part_of UBERON:8930158 ! tonsil secondary follicle.
🔵 SUGGESTION (non-blocking): all 7 new dc-contributor lines still lack the ! <NAME> comment CLAUDE.md's template calls for.
Full details in the PR comment thread.
tonsil germinal center mantle zone (UBERON:8930157): the logical definition used part_of tonsil (UBERON:0002372) while the text definition says "a part of the secondary follicle of the tonsil". The broad differentia risks over-classifying any secondary follicle corona that is part of a tonsil under this term. Change the differentia to part_of tonsil secondary follicle (UBERON:8930158) so the logical definition mirrors the text definition, and drop the now-redundant asserted part_of UBERON:8930158. ELK: no unsatisfiable classes; no new inferred subclasses of UBERON:8930157. Signed: @ai4c-agent Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Claude finished @dosumis's task in 1m 40s —— View job Review checklist
Status: the last remaining 🟡 IMPORTANT issue is now fixedCommit i.e. the genus-differentia logical definition now mirrors the text definition ("A part of the secondary follicle of the tonsil...") instead of the broader Re-verified the rest of the PR directly against
🔵 SUGGESTION (non-blocking, unchanged from prior rounds)All 7 Not independently verifiable in this environment
RecommendationApproving. The 🔴 CRITICAL build-registration blocker (resolved several rounds ago via hand-edit into |
There was a problem hiding this comment.
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
- No obvious CI, auth, or workflow regression
Commit ff8bb10 fixes the last outstanding 🟡 IMPORTANT issue: UBERON:8930157 (tonsil germinal center mantle zone) now has intersection_of: part_of UBERON:8930158 ! tonsil secondary follicle, mirroring its text definition, with the redundant asserted part_of relationship correctly dropped. Re-verified all 7 terms (UBERON:8930153–8930159) directly against uberon-edit.obo — everything else from prior review rounds remains correctly applied (hand-converted stanzas, no stale ODK template registration, palatine tonsil follicle rename, FMA:55221 removal, located_in for the venous plexus).
Only a non-blocking 🔵 suggestion remains: the dc-contributor lines lack the ! <NAME> comment CLAUDE.md's template calls for. Cosmetic only.
Full details in the PR comment thread.
|
#gogoeditdiff |
…nsil-ntr # Conflicts: # src/ontology/uberon-edit.obo
|
Claude finished @dosumis's task in 44s —— View job Review of PR #3719: HRA palatine-tonsil terms (UBERON:8930153–8930159)
I re-checked the 7 stanzas in Findings
🔵 SUGGESTION (non-blocking): the Not verified here: I did not run Recommendation: no 🔴 or 🟡 issues remain. I recommend approval. I can't submit a formal approval myself. |
Here's a diff of how these changes impact the classified ontology (on -base file):Ontology comparisonLeft
Right
Ontology importsOntology annotationspalatine tonsil follicle
|
This branch has not been deployed
4 participants
Generated via bulk_ntr_workflow from source_data/palatine-tonsil.csv.
Pipeline completed through Stage 4:
Parent mapping handled in Stage 3 for mixed source parents:
Final template has no [PENDING], INFER, NEEDS_MAPPING, or UNRESOLVABLE markers.