PlantAI-dev/.superpowers/sdd/task-1-review.md

Task 1 Independent Review

Verdicts

• Specification compliance: NOT COMPLIANT — REVISE. Most required interfaces and behaviors are present, but the versionless-convergence invariant can be bypassed when the TSV also contains an exact unversioned accession.
• Code quality: REVISE. The implementation is readable, standard-library-only, species-scoped, and confined to the requested parser boundary, but the lookup branch ordering creates an important correctness hole and the focused tests do not cover it.

Critical findings

None.

Important findings

1. An exact unversioned key bypasses ambiguity checking for matching versioned accessions. In ReferenceAliasIndex.candidates, lines 242–244 return _exact[query] before lines 246–259 inspect _versionless[query]. Thus a map containing XP_1 -> GeneID 1, XP_1.1 -> GeneID 1, and XP_1.2 -> GeneID 2 returns the aliases for exact key XP_1 instead of raising ambiguous_versionless. The specification grants primary status to exact versioned accessions, while requiring a versionless lookup to converge across every matching version. The exact-first branch currently grants the same priority to an unversioned key and violates that invariant. Restrict the unconditional exact path to version-suffixed queries, or reconcile an exact unversioned row with every matching version before returning. Add a regression test where an unversioned accession is present alongside conflicting versioned rows; the existing collision test omits the exact unversioned row and therefore cannot expose this branch-order defect.

Minor findings

1. Versionless ambiguity details omit the evidence that caused symbol-only conflicts. The ambiguous_versionless error includes matching_accessions and gene_ids, but not gene_symbols, the source path, or declared species. When versions lack numeric GeneIDs and diverge only by gene symbol, details["gene_ids"] is empty and callers cannot report the conflicting entity evidence without reconstructing the index. Retain index provenance and include at least gene_symbols (ideally path/species as well) in the structured error. A symbol-only versionless-conflict test would protect this diagnostic behavior.

Evidence and coverage assessment

• Fresh command: /home/xia-lab/miniconda3/envs/test4py311/bin/python -m pytest tests/unit/test_id_resolution.py -q
• Fresh result: exit code 0; 10 passed in 0.02s. Pytest startup emitted the implementer-reported unrelated RequestsDependencyWarning.
• A direct in-memory probe of the branch above returned ['XP_1', '1'] for a versionless key whose .1 and .2 versions had conflicting GeneIDs, confirming the important finding.
• The required legacy XP ordering, immutability, declared/mixed-species rejection, ordinary version collision, duplicate-row deduplication, explicit GeneID column, structured missing-file/column errors, and lack of substring inference are covered by the focused suite.
• Complete-file inspection found no online query, external dependency, canonical-member rewrite, Ortholog-as-alias behavior, or PPI execution change. The implementation remains job-local and the reviewed scope is surgical.