Repository navigation
add hra brain terms - #3712
add hra brain terms#3712zhengj2007 wants to merge 8 commits into
Conversation
|
Claude finished @zhengj2007's task in 4m 35s —— View job PR Review: add hra brain terms
|
| 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.
![ai4c-agent[bot]](https://avatars.githubusercontent.com/in/2242960?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds 12 new HRA brain terms but has several issues blocking merge:
🔴 CRITICAL:
- 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.
- Typo in term name: 'retrospleninal' → 'retrosplenial' throughout.
- 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.
|
Had a look on terms. Some are also present in HOMBA and well characterised: see https://alleninstitute.github.io/CCF-MAP/docs/HOMBA_ontology_v1.html Others such as ifs1-4 are not used and I would not include them at this stage. There are also other terms that do not have really good supporting evidence and are transition zones between two different regions. |
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 3m 35s —— View job PR Review: add hra brain terms (re-review after new commit)
|
| Term | ID | Line |
|---|---|---|
| extended amygdala | UBERON_9920005 | 6 |
| occipitoparietal transition region | UBERON_9920015 | 13 |
| occipitotemporal transition region | UBERON_9920017 | 14 |
This fails QC checklist item 2 ("Every term has a real PMID/DOI — ASCTB-TEMP placeholder IRIs do not count").
🟡 IMPORTANT
4. UBERON_9920004 overlap with existing UBERON:0013531 (retrosplenial region) still undocumented
candidates.tsv flags this as a possible_match — both cover Brodmann areas 26/29/30. Verified UBERON:0013531 is an active term (is_a: regional part of brain, part_of: cingulate cortex, synonym "retrosplenial cortex"). The gyrus-vs-cortex distinction (paralleling UBERON:0002756 anterior cingulate gyrus vs UBERON:0009835 anterior cingulate cortex) is reasonable, but as long as issue #1 (PENDING) is unresolved this can't be assessed in the actual template row, and no disambiguating xref/relationship to UBERON:0013531 has been added.
5. New reviewer feedback (RiveraAndrea83, 2026-06-15) raises additional scope concerns not yet addressed:
- Several of these terms are already present and better characterized in HOMBA (referencing https://alleninstitute.github.io/CCF-MAP/docs/HOMBA_ontology_v1.html) — worth checking for alignment/reuse rather than independent definitions.
ifs1–ifs4(the four inferior frontal sulcus subdivisions) are described as "not used" and the reviewer suggests excluding them at this stage.- Some terms are transition zones between two regions with weak supporting evidence (consistent with issue 2 other dangling references #3 above —
occipitoparietal/occipitotemporal transition regionhave no real citation).
This PR should not merge until a curator decides how to weigh this domain-expert feedback against the current term list — it may mean dropping ifs1–ifs4 and/or the two transition-region terms rather than sourcing new PMIDs for them.
6. ifj1/ifj2 remain unresolved — correctly excluded from the template, still flagged REQUIRES_CLARIFICATION_FROM_SOURCE in manual_curation.tsv. Should be tracked as a follow-up with ASCTB data providers, not a blocker.
🔵 SUGGESTIONS
7. ifs1–ifs4 source labels not retained as synonyms — name_corrections.tsv recommends keeping them as exact_synonym for atlas-to-ontology mapping; the template has no synonym column populated. (Moot if issue #5 leads to dropping these terms instead.)
8. PR still has no description — a summary of what's added/excluded and the curatorial reasoning (especially for the retrosplenial gyrus/cortex distinction) would materially help review.
Summary
Hierarchy/parents for the other 9 terms remain anatomically consistent and no obsolete terms are referenced. However, this PR is not mergeable as-is: the template still contains an unloadable [PENDING] row with a typo in the term name, three terms lack real literature support, and new domain-expert feedback suggests the term list itself (ifs1–4, transition regions) may need to shrink rather than just gain citations. Requesting changes again — the prior review's blocking items are unchanged since May 25.
|
Reports relocated. Route: manual hand-edit (~12 terms, <20). TODO: per @RiveraAndrea83, drop Automated update (@cellsemantic / dragon-ai-agent, for @dosumis). Reports were relocated out of |
There was a problem hiding this comment.
Re-review after the latest commit (877206a), which only relocated report artifacts out of src/templates/. All previously-flagged blocking issues remain unresolved:
CRITICAL:
- UBERON_9920004 "cingulate gyrus, retrospleninal part" still has a [PENDING] definition and empty is_a/part_of — cannot be loaded by ROBOT. Resolution values already exist in bulk_ntr_workflow/outputs/definitions/uberon_0002967.json.
- Typo "retrospleninal" → "retrosplenial" still present throughout.
- extended amygdala, occipitoparietal transition region, and occipitotemporal transition region still have only ASCTB-TEMP placeholder IRIs as def_xref — no real PMID/DOI.
IMPORTANT:
4. Overlap between UBERON_9920004 and existing UBERON:0013531 (retrosplenial region) still undocumented — no disambiguating xref added.
5. New reviewer feedback (RiveraAndrea83) flags that several terms overlap with better-characterized HOMBA terms, that ifs1–ifs4 are "not used" and should likely be dropped at this stage, and that the transition-region terms lack solid supporting evidence — this needs curator resolution before merge, and may mean shrinking the term list rather than just sourcing citations.
6. ifj1/ifj2 correctly excluded but still need follow-up with ASCTB data providers.
Full details in the PR comment.
|
Claude finished @zhengj2007's task in 2m 38s —— View job PR Review: add hra brain terms (re-review, 3rd pass)
|
There was a problem hiding this comment.
Re-review (3rd pass): the branch has had no substantive changes since the 2026-07-09 review — the only new commit is an empty merge from master. All previously-flagged CRITICAL and IMPORTANT issues remain unresolved:
CRITICAL:
- UBERON_9920004 "cingulate gyrus, retrospleninal part" still has a [PENDING] definition and empty is_a/part_of — cannot be loaded by ROBOT. Resolution values already exist in bulk_ntr_workflow/outputs/definitions/uberon_0002967.json (verified is_a: UBERON:0000200 and part_of: UBERON:0002967 are both active terms).
- Typo "retrospleninal" → "retrosplenial" still present throughout.
- extended amygdala, occipitoparietal transition region, and occipitotemporal transition region still have only ASCTB-TEMP placeholder IRIs as def_xref — no real PMID/DOI.
IMPORTANT:
4. Overlap between UBERON_9920004 and existing UBERON:0013531 (retrosplenial region) is confirmed as a real concern (both cite Brodmann areas 26/29/30) and remains undocumented/undisambiguated.
5. Maintainer/domain-expert feedback (RiveraAndrea83, cellsemantic) to drop ifs1-4 and weak transition-zone terms and check for HOMBA duplicates has not been actioned in the template.
6. ifj1/ifj2 correctly excluded but still need ASCTB follow-up (non-blocking).
Full details in the PR comment and inline comments on src/templates/hra-brain.template.tsv.
- remove cingulate gyrus, retrosplenial part pending expert clarification - add PMIDs for extended amygdala and occipito transition regions - reassign IDs starting from UBERON:8930234
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Removed the folder bulk_ntr_workflow/outputs/definitions, including all files within it.
Updated src/templates/hra-brain.template.tsv
|
|
Claude finished @zhengj2007's task in 6m 0s —— View job Re-review after latest push (4th pass, after commits 1416520 / 36b589d)Todo list:
ResultGood progress since the last review — all three previously-blocking 🔴 CRITICAL issues are now resolved:
However, I still had to request changes (🟡 IMPORTANT, full review posted here):
Plus a few 🔵 suggestions (stale typo in leftover workflow-report artifacts, an empty Note on tooling: I was unable to verify the new PMIDs' actual content in this pass — all literature-lookup tools (web search, Europe PMC MCP tools, direct curl) required permissions that weren't granted in this environment/run. If PMID verification is needed, please re-run with those tools enabled or check manually. |
![ai4c-reviewer[bot]](https://avatars.githubusercontent.com/in/4388239?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review: add hra brain terms (re-review, 4th pass — after commits 1416520 / 36b589d)
Review checklist:
- Hierarchy and parents are consistent (mostly — see IMPORTANT #1 below)
- Definitions and supporting references look adequate (format-wise; content not independently verifiable this pass, see note)
- 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
What changed since the last review
Good progress. The latest commits (1416520, 36b589d) resolve all three previously-flagged 🔴 CRITICAL items:
- ✅
[PENDING]row removed — "cingulate gyrus, retrosplenial part" (formerlyUBERON_9920004) has been dropped entirely fromsrc/templates/hra-brain.template.tsvrather than force-resolving the overlap withUBERON:0013531. Reasonable resolution given it needed domain-expert clarification. - ✅ Typo moot — "retrospleninal" no longer appears in the loaded template (the term is gone). It's still present in the leftover workflow-report artifacts (
bulk_ntr_workflow/outputs/hra-brain-reports/input.tsv,candidates.tsv) — harmless since these aren't consumed by ROBOT, see 🔵 suggestion below. - ✅ Real PMIDs added for extended amygdala (
PMID:12724159), occipitoparietal transition region (PMID:39976051), occipitotemporal transition region (PMID:39976051). - ✅ ID range confirmed legitimate:
xsd:integer[>= 8930000, < 8931000]is allocated to "Jie Zheng" asidrange:42insrc/ontology/uberon-idranges.owl, and none ofUBERON:8930234–8930244collide with existing terms inuberon-edit.obo. Using definitive IDs directly (rather thanUBERON:99xxxxxtemp IDs) matches the documented workaround inbulk_ntr_workflow/template-id-allocation-ticket.mdfor template-based term minting, and is consistent with the siblingadd-hra-fallopian_tube-ntrPR.
🟡 IMPORTANT (new/carried-over)
1. extended amygdala (UBERON_8930236, line 5) is part_of: UBERON:0001893 (telencephalon) — anatomically too coarse.
UBERON:0001893 is telencephalon, not amygdala. The term's own definition says it comprises "the central medial amygdala, sublenticular substantia innominata, and bed nucleus of the stria terminalis" — and UBERON:0002743 (basal forebrain) already exists with a definition that explicitly enumerates "amygdala" and "substantia innominata" as constituents, making it a much more precise part_of parent than the very broad telencephalon (nearly everything in the forebrain is transitively part_of telencephalon, so this adds little specificity). Note the workflow's own bulk_ntr_workflow/outputs/hra-brain-reports/input.tsv originally proposed UBERON:0001876 (amygdala) as the parent before curation changed it to telencephalon — worth double-checking this was an intentional, reasoned change rather than a curation slip. The sibling term "septal nuclei" in this same batch sits in basal-forebrain territory, reinforcing that basal forebrain is the more natural home here.
2. PMID:39976051 is reused as def_xref for two different terms — "occipitoparietal transition region" (line 12) and "occipitotemporal transition region" (line 13). Reusing the identical citation for two distinct anatomical boundary regions is unusual; please confirm this single paper genuinely discusses both transition zones (vs. only one, with the other citation added as a placeholder-of-convenience). I attempted to independently verify this and PMID:12724159 via Europe PMC / web search in this review pass, but all outbound lookup tools (WebSearch, mcp__artl-mcp__*, direct curl to Europe PMC/PubMed) were blocked by environment permissions in this run — this needs a manual check by a human curator or a review pass with those tools enabled.
3. ifs1–ifs4 (anterior/middle-anterior/middle-posterior/posterior inferior frontal sulcus, lines 8–11) are still present, and the maintainer TODO to drop them is still unaddressed. @RiveraAndrea83 (2026-06-15) flagged these as "not used," and @cellsemantic's maintainer TODO (2026-07-09) explicitly said to "drop ifs1-4 and weak transition-zone terms before adding; some terms are already well-characterised in HOMBA — check for duplicates first." Neither the ifs1-4 removal nor a HOMBA duplicate-check appears to have happened — the two transition-region terms (occipitoparietal/occipitotemporal, also flagged as weakly-supported) also remain unchanged. Compounding this: the four ifs1-4 definitions are identical boilerplate ("A regional part of the inferior frontal sulcus, representing a parcellated anatomical subdivision used in neuroanatomical mapping and brain atlases.") that doesn't actually differentiate anterior from posterior, and all four share the same single PMID:17584753 citation.
4. ifj1/ifj2 remain correctly excluded, still tracked in manual_curation.tsv as REQUIRES_CLARIFICATION_FROM_SOURCE — non-blocking, just a reminder this follow-up is still open.
🔵 SUGGESTIONS
5. Leftover "retrospleninal" typo and the now-obsolete possible_match row for "cingulate gyrus, retrosplenial part" in bulk_ntr_workflow/outputs/hra-brain-reports/{input.tsv,candidates.tsv} could be cleaned up now that the term was dropped, to avoid confusing future readers of these historical artifacts.
6. bulk_ntr_workflow/outputs/template_groups_initial.tsv is just a two-line header stub with no data rows — likely fine to remove since no term groups ended up being used in this batch.
7. PR still has no description — a summary of what was added/excluded/changed in curation (especially explaining the retrosplenial-term drop and the extended-amygdala parent choice) would help future reviewers.
Summary
Substantial progress: all three previously-blocking 🔴 CRITICAL issues are resolved, and the new ID range and [PENDING]-row removal both check out. However, this still isn't mergeable as-is: the maintainer's explicit TODO to drop ifs1-ifs4 and weak transition-zone terms / check HOMBA for duplicates hasn't been actioned, the extended amygdala part_of parent looks anatomically imprecise given a better-fitting existing term (basal forebrain) is available, and the reused PMID:39976051 citation needs curator confirmation. Requesting changes again, but the remaining list is much shorter than in prior passes.
Remove dropped ifs1-ifs4 records from the template, update extended amygdala part_of to UBERON:0001876, delete obsolete template_groups_initial.tsv, and clean historical hra-brain report artifacts by fixing retrosplenial typo and removing obsolete possible_match row. Signed-off-by: @ai4c-agent
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
This PR modifies some logical definitions. See details below:Number of classes with modified logical definitions: 2 http://purl.obolibrary.org/obo/UBERON_1200342Removed subclasses: 1 http://purl.obolibrary.org/obo/UBERON_1200343No changes in inferred subclasses. |
|
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. |
|
Claude finished @zhengj2007's task in 1m 11s —— View job PR Review: add hra brain terms (5th pass)
The branch is now down to 8 new terms ( I did not verify the PMIDs in this pass. Literature lookups were not run. 🟡 IMPORTANT
🔵 SUGGESTIONS
SummaryThe earlier blockers are fixed. I would still ask for changes until the transition-region terms are either dropped or justified, the duplicated PMID is confirmed, and the HOMBA check is documented. I did not submit a formal review status. |
This branch has not been deployed
4 participants
Uh oh!
There was an error while loading. Please reload this page.