Skip to content

add hra brain terms#3712

Open
zhengj2007 wants to merge 1 commit into
masterfrom
add-hra-brain-ntr
Open

add hra brain terms#3712
zhengj2007 wants to merge 1 commit into
masterfrom
add-hra-brain-ntr

Conversation

@zhengj2007
Copy link
Copy Markdown
Collaborator

No description provided.

@ai4c-agent
Copy link
Copy Markdown

ai4c-agent Bot commented May 25, 2026

Claude finished @zhengj2007's task in 4m 35s —— View job


PR Review: add hra brain terms

Review checklist:

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

Overview

This PR adds 12 new HRA (Human Reference Atlas) brain terms via ROBOT template (src/templates/hra-brain.template.tsv), along with the bulk NTR workflow intermediate files (JSON definitions, report TSVs). The workflow correctly identified and excluded 4 confirmed existing terms (amygdaloid complex, septal nuclei, thalamus, occipitotemporal fusiform gyrus occipital part) and correctly excluded 2 unresolvable terms (ifj1, ifj2) that need clarification from the ASCTB data providers.

However, there are several issues that need to be resolved before this can be merged.


🔴 CRITICAL

1. UBERON_9920004 has [PENDING] definition and empty hierarchy (src/templates/hra-brain.template.tsv, line 5)

"cingulate gyrus, retrospleninal part" was not fully processed — the template row has [PENDING] as the definition and empty is_a/part_of columns. This violates QC checklist item 1 ("No [PENDING] definitions remain"). The term cannot be loaded by ROBOT.

The JSON subagent output (bulk_ntr_workflow/outputs/definitions/uberon_0002967.json) does have a resolution:

  • Definition: "A subdivision of the cingulate gyrus located in its posterior-most region, corresponding to the retrosplenial cortical areas and comprising Brodmann areas 26, 29, and 30."
  • is_a: UBERON:0000200 (gyrus)
  • part_of: UBERON:0002967 (cingulate gyrus)
  • def_xref: PMID:17572666
  • xref: Wikipedia:Retrosplenial_cortex

These values need to be applied to the template row before merge. Fix this →

2. Typo in term name (src/templates/hra-brain.template.tsv, line 5)

cingulate gyrus, retrosplen**in**al part → should be cingulate gyrus, retrosplen**i**al part. The JSON output file (uberon_0002967.json) has the correct spelling "retrosplenial". This typo is present throughout the template (label column) and the bulk_ntr_workflow/outputs/definitions/input/uberon_0002967.json and src/templates/hra-brain-reports/input.tsv files. Fix this →

3. Three terms lack real literature references in def_xref (QC checklist item 2: "Every term has a real PMID/DOI — ASCTB-TEMP placeholder IRIs do not count"):

Term ID Current def_xref
extended amygdala UBERON_9920005 ASCTB-TEMP IRI only
occipitoparietal transition region UBERON_9920015 ASCTB-TEMP IRI only
occipitotemporal transition region UBERON_9920017 ASCTB-TEMP IRI only

Suggested references:

  • extended amygdala: PMID:25899707 (cited in related manual_curation.tsv for IFJ; a dedicated extended amygdala reference such as PMID:12948448 should be verified)
  • occipitoparietal transition region: JSON unresolvable section suggests PMID:25316338 or PMID:28981612
  • occipitotemporal transition region: JSON suggests PMID:19457377

🟡 IMPORTANT

4. Possible overlap with existing UBERON:0013531 (retrosplenial region) — curator decision needed

The candidates.tsv flags "cingulate gyrus, retrosplenial part" as a possible_match for UBERON:0013531. Both terms cover the same Brodmann areas (26, 29, 30) and the same anatomical territory. The candidates report notes:

"UBERON:0013531 (retrosplenial region) refers to the same anatomical location but is classified as a cortical region (is_a: regional part of brain, part_of: cingulate cortex), while the requested term would be a gyral subdivision (is_a: gyrus, part_of: cingulate gyrus). Similar dual gyrus/cortex terminology exists in UBERON for anterior cingulate gyrus (UBERON:0002756) vs anterior cingulate cortex (UBERON:0009835)."

This is the correct reasoning for allowing both terms. However, the PR should document this decision, and the new term should add an xref or relationship to UBERON:0013531 for disambiguation.

5. ifj1 and ifj2 remain unresolved — labels are not anatomically standard

These are correctly excluded from the final template (good), but they are still present in the source data (hra-brain-reports/errors.tsv). Per the manual_curation.tsv, the ASCTB data providers need to clarify what anatomical/functional features distinguish ifj1 from ifj2, and provide standard anatomical names. Should be tracked as a follow-up action.


🔵 SUGGESTIONS

6. ifs1–ifs4 source labels not retained as synonyms

The name_corrections.tsv recommends keeping the source labels (ifs1, ifs2, ifs3, ifs4) as exact synonyms since these are atlas-specific codes. The template currently only shows the corrected names (anterior/middle-anterior/middle-posterior/posterior inferior frontal sulcus) with no synonym column. Consider adding these as exact_synonym for atlas-to-ontology mapping.

7. PR has no description

The PR body is empty. A brief description of what terms are added, what was excluded and why, and what decisions were made during curation would aid review significantly.

8. bulk_ntr_workflow/ intermediate files

These files appear to be workflow artifacts (JSON inputs/outputs, initial TSVs). It's worth confirming whether these should be committed to the repository long-term or are only needed during the curation workflow.


Summary

Verdict: Request changes. The template has an incomplete entry (UBERON_9920004 with [PENDING] definition and no hierarchy) and a typo in the same term's name, plus three terms without real literature references — all failing the QC checklist required before merge. The rest of the new terms (atrium of lateral ventricle, body/head of hippocampus, frontomarginal gyrus, extended amygdala, 4 inferior frontal sulcus subdivisions, 2 transition regions) look anatomically reasonable and well-structured.

Copy link
Copy Markdown

@ai4c-agent ai4c-agent Bot left a comment

Choose a reason for hiding this comment

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

This PR adds 12 new HRA brain terms but has several issues blocking merge:

🔴 CRITICAL:

  1. UBERON_9920004 'cingulate gyrus, retrosplenial part' has a [PENDING] definition and empty is_a/part_of hierarchy — cannot be loaded by ROBOT. The resolved values exist in bulk_ntr_workflow/outputs/definitions/uberon_0002967.json and need to be applied to the template.
  2. Typo in term name: 'retrospleninal' → 'retrosplenial' throughout.
  3. Three terms (extended amygdala, occipitoparietal transition region, occipitotemporal transition region) have only ASCTB-TEMP placeholder IRIs as def_xref — real PMIDs/DOIs required per QC checklist.

🟡 IMPORTANT:
4. UBERON_9920004 possible overlap with existing UBERON:0013531 (retrosplenial region) — curator decision should be documented and a cross-reference added.
5. ifj1/ifj2 correctly excluded but need follow-up with ASCTB data providers for proper naming.

🔵 SUGGESTIONS:
6. ifs1–ifs4 source labels should be retained as exact synonyms.
7. PR needs a description explaining what was added, excluded, and why.

See detailed review in the PR comment.

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.

1 participant