galaxyproject / galaxyproject/brc-analytics

Fix MCP-server CatalogData lineage index polluting ancestor keys with descendants

Open Beginner friendly
#1,346 0 comments 0 reactions 0 assignees View on GitHub
refactor
Dominant language
TypeScript
Stars
7
Forks
11
Avg merge
2d 12h
Merged PRs (30d)
16

Description

## Summary
The MCP server's `CatalogData._build_lineage_index` (`backend/api/app/services/catalog_data.py`) maps **every** taxon ID in a genome's lineage to that genome's **full** lineage set, so ancestor keys end up containing their own descendants. This is the same bug that #1328 fixed in the assistant's copy (`app/services/tools/catalog_data.py`), but the MCP copy was never updated.

## Where
`backend/api/app/services/catalog_data.py`, lines ~76–81:
```python
for tid in lineage_strs:
existing = self._lineage_by_tax_id.get(tid)
if existing is None:
self._lineage_by_tax_id[tid] = set(lineage_strs) # full lineage → pollutes ancestor keys
else:
existing.update(lineage_strs)
```
Because `lineageTaxonomyIds` is root-first, key `"2"` (Bacteria) ends up containing `"562"` (E. coli). `get_compatible_workflows` then asks "is the workflow's taxon in the target's lineage?" against this index — so for a high-rank *target* organism it would wrongly match a workflow that targets a *descendant* taxon.

## Impact
**Latent, not currently triggered.** No catalog organism is a high-rank taxon today, so every lookup target is a leaf whose index entry is correct. The bug only manifests if/when a higher-rank taxon becomes a lookup target. Flagged during review of #1328; see that PR's discussion.

## Fix
Mirror #1328's assistant-copy fix — index each taxon to its own ancestor prefix (`root..tid`) instead of the full lineage:
```python
for i, tid in enumerate(lineage_strs):
ancestors = set(lineage_strs[: i + 1])
existing = self._lineage_by_tax_id.get(tid)
if existing is None:
self._lineage_by_tax_id[tid] = ancestors
else:
existing.update(ancestors)
```
Add a regression test analogous to `test_ancestor_index_excludes_descendants` in the assistant copy (assert `"562" not in index["2"]`).

## Notes
This is one facet of the broader 4-way duplication of the compatibility logic tracked in #1327; fixing it here is a small, standalone correction that doesn't need to wait for consolidation. Surfaced in review of #1328.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in backend/api/app/services/catalog_data.py at _build_lineage_index and compare its behavior with the assistant-copy fix from #1328. Add a regression test analogous to test_ancestor_index_excludes_descendants, then run the relevant catalog-data tests and verify that an ancestor index entry excludes descendant taxon IDs.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.