Skip to content

lstrip("PMCpmc") in pubmed_search.py strips characters, not a prefix #127

Description

@Bug-Keeper

Pretty sure this is a .lstrip() semantics confusion. In skills/sn-search-academic/scripts/pubmed_search.py:119:

pmc_id = id_elem.text.lstrip("PMCpmc").strip() or id_elem.text

The comment one line up says "去掉 'PMC' 前缀,只保留数字" — strip the PMC prefix and keep only digits. But str.lstrip(chars) doesn't strip a prefix; it strips any leading run of characters contained in chars. So if PubMed ever returned an ID with mixed casing or any leading char that happens to be in "PMCpmc", the result would be wrong:

  • "PMC123""123" ✓ (by coincidence)
  • "PMCC456""456" ✓ (because C is in the set)
  • "pmpc789""789" — also stripped, but that's not a real prefix
  • A hypothetical "PMP500""500" — bug surface in disguise

In practice today PMC IDs are always PMC + digits, so the bug doesn't fire — but removeprefix expresses intent and is robust:

text = id_elem.text or ""
pmc_id = text.removeprefix("PMC").removeprefix("pmc").strip() or text

Severity: Low (latent — current PMC IDs are well-formed) but worth fixing because the wrong abstraction here is the kind of thing that bites two years later when the API adds a new ID variant.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions