Replace hardcoded parser with PubChem structure parser#37
Merged
Conversation
- Replace hardcoded molecule parsing tables with PubChem CID and record-based parsing\n- Use real atom/bond coordinates from PubChem records with explicit parse errors\n- Keep a small offline fallback set for common molecules\n- Fix SMILES lookup and parser cache key handling to preserve case sensitivity\n- Update README to document the new parsing approach and limitations\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous hardcoded/pattern-based molecule parsing with a PubChem-backed structure loader that resolves identifiers to PubChem CIDs, fetches compound records (preferring 3D), and builds atoms/bonds directly from returned coordinate/bond data. It also updates identifier lookup to preserve SMILES case sensitivity and refreshes README documentation accordingly.
Changes:
- Reworked
parseMolecule()to resolve identifiers → PubChem CID → PubChem compound record (3D-first) →Molecule3D. - Added parsing cache and an explicit error path when no structure data is available, plus a small offline fallback set.
- Updated local identifier lookup (SMILES case sensitivity) and README to reflect the new architecture/limitations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/utils/moleculeParser.ts | Replaces hardcoded parsing with PubChem CID resolution + record fetching, adds caching and offline fallbacks. |
| src/utils/identifierLookup.ts | Preserves SMILES case in local identifier lookup to avoid aromatic/aliphatic collapsing. |
| README.md | Updates documentation to describe PubChem-backed parsing and current limitations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+42
| 'iupacname:water': createWater(), | ||
| 'iupacname:methane': createMethane(), | ||
| 'iupacname:ammonia': createAmmonia(), | ||
| 'iupacname:carbon dioxide': createCarbonDioxide(), | ||
| 'iupacname:benzene': createBenzene(), |
Comment on lines
+44
to
+48
| 'casnumber:7732-18-5': createWater(), | ||
| 'casnumber:74-82-8': createMethane(), | ||
| 'casnumber:7664-41-7': createAmmonia(), | ||
| 'casnumber:124-38-9': createCarbonDioxide(), | ||
| 'casnumber:71-43-2': createBenzene(), |
| 'casnumber:7664-41-7': createAmmonia(), | ||
| 'casnumber:124-38-9': createCarbonDioxide(), | ||
| 'casnumber:71-43-2': createBenzene(), | ||
| 'casnumber:64-17-5': createEthanol() |
Comment on lines
+328
to
+333
| function findOfflineFallback(candidates: IdentifierInput[]): Molecule3D | null { | ||
| for (const candidate of candidates) { | ||
| const key = `${candidate.type}:${candidate.value.trim().toLowerCase()}`; | ||
| const fallback = COMMON_OFFLINE_FALLBACKS[key]; | ||
| if (fallback) { | ||
| return fallback; |
Comment on lines
+93
to
+96
| throw new Error( | ||
| `Unable to parse molecule "${cleanValue}". No structure data was found for the provided identifiers.` | ||
| + (failureMessages.length > 0 ? ` Last error: ${failureMessages[0]}` : '') | ||
| ); |
Comment on lines
+127
to
+135
| const unique = new Map<string, IdentifierInput>(); | ||
| for (const entry of [primary, ...fromIdentifiers]) { | ||
| const key = getIdentifierKey(entry.type, entry.value); | ||
| if (!unique.has(key)) { | ||
| unique.set(key, entry); | ||
| } | ||
| } catch (error) { | ||
| console.warn('Failed to lookup SMILES from PubChem:', error); | ||
| } | ||
|
|
||
| // Try partial matches for common patterns | ||
| const partialMatch = tryPartialMatches(lowerName); | ||
| if (partialMatch) return partialMatch; | ||
|
|
||
| // Pattern matching for alkenes | ||
| const alkeneMatch = tryMatchAlkene(lowerName); | ||
| if (alkeneMatch) return alkeneMatch; | ||
|
|
||
| // Pattern matching for alkanes | ||
| const alkaneMatch = tryMatchAlkane(lowerName); | ||
| if (alkaneMatch) return alkaneMatch; | ||
| return [...unique.values()]; |
Comment on lines
+31
to
+37
| const COMMON_OFFLINE_FALLBACKS: Record<string, Molecule3D> = { | ||
| 'smiles:o': createWater(), | ||
| 'smiles:c': createMethane(), | ||
| 'smiles:n': createAmmonia(), | ||
| 'smiles:o=c=o': createCarbonDioxide(), | ||
| 'smiles:c1ccccc1': createBenzene(), | ||
| 'smiles:cco': createEthanol(), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The previous molecule parser relied on a large hardcoded lookup table and could silently return incorrect structures (for example, showing methane or benzene for unrelated valid inputs). This change replaces that behavior with structure parsing based on real compound records.
What changed
moleculeParser.tswith a PubChem-driven flow:C1CCCCC1(cyclohexane) from collapsing intoc1ccccc1(benzene).Notes for review
eslint.config.*), unrelated to this change.Fixes: #35