refactor: modularize GitHub metadata import pipeline - #53
Conversation
Split importer responsibilities into focused modules for author matching, contributor retrieval, file parsing, and shared normalization utilities. Centralized author and repository normalization logic to reduce duplication and improve maintainability. Updated citation validation tests to verify required author-field handling remains correct after the refactor.
…orter and contributors
There was a problem hiding this comment.
Pull request overview
This PR refactors the GitHub metadata import pipeline by extracting shared normalization utilities and splitting contributor fetching, file parsing, author enrichment/deduping, and metadata merging into focused modules, while updating tests to protect key behaviors.
Changes:
- Introduced shared GitHub importer utilities (
githubImporterUtils) plus dedicated modules for parsing, merging, author handling, and contributor retrieval. - Updated
githubImporterto use the new modules and GitHub API URL/config builders. - Expanded/adjusted test coverage around the refactor boundaries (request config shape, quote stripping, and citation export expectations).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/services/githubImporter.test.js | Adds regression tests for new helpers (buildGithubRequestConfig, stripWrappingQuotes). |
| tests/services/citationValidation.test.js | Updates citation validation test input to match required author shape in toCitationCff. |
| src/services/githubImporterUtils.js | New shared normalization/cleanup utilities used across importer modules. |
| src/services/githubImporterParsers.js | New module for parsing repository metadata files (package.json, TOML, setup.py, pom.xml, README). |
| src/services/githubImporterMerge.js | New module to merge parsed sources + contributor data into canonical metadata and warnings. |
| src/services/githubImporterContributors.js | New module to fetch and normalize contributor-based fallback authors (incl. ORCID enrichment). |
| src/services/githubImporterAuthors.js | New module for author matching/enrichment/deduping and contributor-rank ordering. |
| src/services/githubImporter.js | Refactors importer orchestration to use modularized helpers and shared utilities. |
| src/services/githubApi.js | Adds API URL builders + request config helper used by importer and legacy GitHub service. |
| src/services/github.js | Updates legacy GitHub service to use new API URL builders. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Split importer responsibilities into focused modules for author matching, contributor retrieval, file parsing, and shared normalization utilities. Centralized author and repository normalization logic to reduce duplication and improve maintainability. Updated citation validation tests to verify required author-field handling remains correct after the refactor.
…orter and contributors
3af823b to
232efb8
Compare
…ics/OpenCite into refactor/github-importer
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/services/githubImporter.js:66
extractFirstMarkdownParagraphis assigned here but never used after the refactor, so it can be removed to avoid misleading future edits.
const stripWrappingQuotes = utilStripWrappingQuotes;
const extractFirstMarkdownParagraph = utilExtractFirstMarkdownParagraph;
src/services/githubImporter.js:530
importGithubMetadatahard-codes the GitHub repo API URL while the rest of the refactor introduces URL builder helpers (e.g.,buildGithubReleaseApiUrl). UsingbuildGithubRepoApiUrl(owner, repo)would keep GitHub API access centralized and reduce duplication.
const repoData = await fetchRequiredJson(`https://api.github.com/repos/${owner}/${repo}`, {
authToken,
source: 'repository',
onError: (source, code, message, details = {}) => addError(errors, source, code, message, details),
});
src/services/githubImporter.js:28
utilNormalizeGrantsandutilNormalizeVersionForCompareare imported fromgithubImporterUtils.jsbut are unused in this module, which adds dead code and makes refactors harder.
This issue also appears on line 65 of the same file.
normalizeGrants as utilNormalizeGrants,
normalizeKeywords as utilNormalizeKeywords,
normalizeReferences as utilNormalizeReferences,
normalizeRepoUrl as utilNormalizeRepoUrl,
normalizeVersionForCompare as utilNormalizeVersionForCompare,
src/services/githubImporterMerge.js:61
primaryAuthorsis already normalized vianormalizeAuthors(...), so callingnormalizeAuthors(primaryAuthors)again is redundant work and can subtly re-normalize casing/spacing. Reuse the normalized list directly.
const authors = [
...normalizeAuthors(primaryAuthors),
...normalizeAuthors(Array.isArray(contributors) ? contributors : []),
];
egrace479
left a comment
There was a problem hiding this comment.
A few notes on code efficiency and question on checks.
I tested on a couple repos and observed that on https://github.com/Imageomics/got-milk-monarchs, it didn't pick up Net (I'm wondering if this is because his commits are tied to Claude's, suggesting that author eval maybe isn't handling co-author commits well).
Another note--not necessarily for this PR--is that the warnings for required fields are showing up before anything has been entered. I think including an asterisk to indicate required fields is fine, but it shouldn't highlight red with a warning until someone interacts with the field.
Similarly, it's a bit unclear to what this section is referring:
Is "Needs attention" the number of empty fields? I assume "done well" is referencing the number that are filled in without error and that "errors" refers to the unfilled required field?
|
I also just tried loading metadata for the catalog and got a bunch of errors.
Console errors:
Those user profile errors (in the console) were also present with the other repos I used for testing. It seems to be trying to call all the files, releases, etc.:
It didn't pick up on the DOI for the catalog either. |
…tracting co-author names from commit messages
…tion and GitHub importer services
There was a problem hiding this comment.
Amazing job at the refactorization process! This must've been a lot of work modularizing everything so this is a great setup. Love all of the features on OpenCite! Here are some of my observations for now listed below while I continue testing other repos:
I went through the repos that Elizabeth has mentioned so far and Net was present in the got-milk-monarchs repo now. with no console errors.
For catalog, I also received the same import warnings but there are no console warnings on my end. However, I noticed the "Reviewed metadata loaded in editor" section has listed several warnings, passing checks, and errors. I'm not sure what those 3 are specifically referencing to, so I think this is something that's more unclear. It said there are 3 warnings for catalog but there are 5 import warnings, unless the warnings were meant for something else?
|
The release date mis-match I'm now getting for the catalog is because the GitHub API uses UTC. We might consider including the UTC time stamp of the release in the warning or a note that the API date is in UTC if it's just off by a day.
As long as @mandylan75's concern about the warning count is addressed (unless it's also a base-branch issue), I think this PR is good to go! @mandylan75, do you see the same issue on |
egrace479
left a comment
There was a problem hiding this comment.
Testing again:
- It didn't pick up Dylan on https://github.com/Imageomics/got-milk-monarchs, probably because he doesn't have his name in GitHub, so that may be considered reasonable?
- I think it's filtering Agents after limiting to 5 contributors, which excludes real people (catalog has more than 3 contributors, but only loads the 3 and states that it "Excluded 3 automated account(s) from fallback authors."
I don't think any of these are blocking issues, but we may document for reference.



Split importer responsibilities into focused modules for author matching, contributor retrieval, file parsing, and shared normalization utilities. Centralized author and repository normalization logic to reduce duplication and improve maintainability. Updated citation validation tests to verify required author-field handling remains correct after the refactor.