Skip to content

refactor: modularize GitHub metadata import pipeline - #53

Merged
beanbean9339 merged 25 commits into
feature/github-metadata-importfrom
refactor/github-importer
Sep 3, 2026
Merged

refactor: modularize GitHub metadata import pipeline#53
beanbean9339 merged 25 commits into
feature/github-metadata-importfrom
refactor/github-importer

Conversation

@beanbean9339

Copy link
Copy Markdown
Collaborator

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.

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 githubImporter to 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.

Comment thread src/services/githubImporterUtils.js
Comment thread src/services/githubImporterMerge.js Outdated
Comment thread src/services/githubImporter.js Outdated
Comment thread src/services/githubImporterParsers.js Outdated
Comment thread src/services/githubApi.js
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.
@beanbean9339
beanbean9339 force-pushed the refactor/github-importer branch from 3af823b to 232efb8 Compare August 14, 2026 21:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • extractFirstMarkdownParagraph is 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

  • importGithubMetadata hard-codes the GitHub repo API URL while the rest of the refactor introduces URL builder helpers (e.g., buildGithubReleaseApiUrl). Using buildGithubRepoApiUrl(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

  • utilNormalizeGrants and utilNormalizeVersionForCompare are imported from githubImporterUtils.js but 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

  • primaryAuthors is already normalized via normalizeAuthors(...), so calling normalizeAuthors(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 egrace479 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Image 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?

Comment thread src/services/githubApi.js Outdated
Comment thread src/services/githubImporter.js Outdated
Comment thread src/services/githubImporterContributors.js Outdated
Comment thread src/services/githubImporterMerge.js Outdated
Comment thread src/services/githubImporterMerge.js Outdated
@egrace479

Copy link
Copy Markdown
Member

I also just tried loading metadata for the catalog and got a bunch of errors.

Screenshot 2026-08-20 at 7 13 36 PM

Console errors:

Screenshot 2026-08-20 at 7 14 07 PM

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.:

Screenshot 2026-08-20 at 7 16 55 PM

It didn't pick up on the DOI for the catalog either.

…tracting co-author names from commit messages
Comment thread src/services/githubImporterContributors.js
@beanbean9339
beanbean9339 requested a review from egrace479 August 27, 2026 17:00

@mandylan75 mandylan75 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Image

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?

image

@egrace479

egrace479 commented Aug 27, 2026

Copy link
Copy Markdown
Member

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 feature/github-metadata-import?

@egrace479 egrace479 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@beanbean9339
beanbean9339 merged commit 7c9c1c3 into main Sep 3, 2026
@beanbean9339
beanbean9339 deleted the refactor/github-importer branch September 3, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants