From 23dc7f315d8cd3dcd6d4cd018a5a6477eb17b06c Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Sat, 26 Sep 2026 22:59:58 -0400 Subject: [PATCH] feat: discover human authors from bounded commit history (#68) --- README.md | 6 ++ src/services/githubApi.js | 5 +- src/services/githubImporter.js | 11 +++- src/services/githubImporterCommitAuthors.js | 63 +++++++++++++++++++++ src/services/githubImporterContributors.js | 1 + tests/services/commitAuthors.test.js | 41 ++++++++++++++ tests/services/githubImporter.test.js | 14 ++--- 7 files changed, 131 insertions(+), 10 deletions(-) create mode 100644 src/services/githubImporterCommitAuthors.js create mode 100644 tests/services/commitAuthors.test.js diff --git a/README.md b/README.md index 8412f16..651cdfd 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,12 @@ During export, OpenCite validates generated `.zenodo.json` metadata. ZIP exports 5. Imported author lists include contributor-based context and are deduplicated. 6. Review, adjust, and regenerate metadata files before release. +The importer also considers human author names in recent commit history when +repository metadata and contributor profiles do not include everyone. It scans +up to 100 commits without a token or 1,000 with a token, omitting bot names +and GitHub handles rather than using them as citation names. Verify imported +authors before exporting citation metadata. + ## Validation Behavior OpenCite validates metadata at multiple stages: diff --git a/src/services/githubApi.js b/src/services/githubApi.js index 9e4376f..038742e 100644 --- a/src/services/githubApi.js +++ b/src/services/githubApi.js @@ -104,10 +104,11 @@ export function buildGithubReleaseListApiUrl(owner, repo, perPage = 1) { return `${API_BASE}/repos/${owner}/${repo}/releases?per_page=${safePerPage}`; } -export function buildGithubCommitListApiUrl(owner, repo, defaultBranch = '', perPage = 1) { +export function buildGithubCommitListApiUrl(owner, repo, defaultBranch = '', perPage = 1, page = null) { const safePerPage = Number.isInteger(perPage) ? Math.min(Math.max(perPage, 1), 100) : 1; const branchFilter = defaultBranch ? `&sha=${encodeURIComponent(defaultBranch)}` : ''; - return `${API_BASE}/repos/${owner}/${repo}/commits?per_page=${safePerPage}${branchFilter}`; + const pageFilter = Number.isInteger(page) && page > 1 ? `&page=${page}` : ''; + return `${API_BASE}/repos/${owner}/${repo}/commits?per_page=${safePerPage}${branchFilter}${pageFilter}`; } export function buildGithubBranchApiUrl(owner, repo, branch) { diff --git a/src/services/githubImporter.js b/src/services/githubImporter.js index 5773f87..f941830 100644 --- a/src/services/githubImporter.js +++ b/src/services/githubImporter.js @@ -33,6 +33,7 @@ import { resolveContributorFallbackLimit, } from './githubImporterContributors.js'; import { dedupeAuthors } from './githubImporterAuthors.js'; +import { fetchCommitAuthors } from './githubImporterCommitAuthors.js'; import { addCitationConsistencyWarnings, mergeMetadata } from './githubImporterMerge.js'; export { addCitationConsistencyWarnings, mergeMetadata }; import { @@ -549,7 +550,7 @@ export async function importGithubMetadata(repoUrl, options = {}) { ); const releaseData = Array.isArray(releaseList) && releaseList.length > 0 ? releaseList[0] : null; const recentCommitPayload = await fetchOptionalJson( - buildGithubCommitListApiUrl(owner, repo, defaultBranch, 10), + buildGithubCommitListApiUrl(owner, repo, defaultBranch, 100), buildGithubRequestConfig({ authToken, source: 'commits', @@ -716,14 +717,22 @@ export async function importGithubMetadata(repoUrl, options = {}) { fetchOptionalJson, extractOrcidFromGithubProfile, }); + const commitAuthors = await fetchCommitAuthors({ + owner, repo, defaultBranch, initialCommits: recentCommitPayload, + knownGithubLogins: contributorResult.githubLogins, + warnings, authToken, cleanString, normalizeAuthor, normalizeAuthors, + addWarning, fetchOptionalJson, + }); const coAuthorAuthors = normalizeAuthors(commitCoAuthorNames.map((name) => normalizeAuthor({ name }))); const contributors = dedupeAuthors([ ...coAuthorAuthors, ...contributorResult.fallbackAuthors.filter(Boolean), + ...commitAuthors, ]); const contributorLookupAuthors = dedupeAuthors([ ...coAuthorAuthors, ...contributorResult.lookupAuthors.filter(Boolean), + ...commitAuthors, ]); addRateLimitHintIfNeeded(warnings, authToken); diff --git a/src/services/githubImporterCommitAuthors.js b/src/services/githubImporterCommitAuthors.js new file mode 100644 index 0000000..6411e04 --- /dev/null +++ b/src/services/githubImporterCommitAuthors.js @@ -0,0 +1,63 @@ +import { buildGithubCommitListApiUrl, buildGithubRequestConfig } from './githubApi.js'; +import { dedupeAuthors } from './githubImporterAuthors.js'; + +const COMMIT_PAGE_SIZE = 100; +const UNAUTHENTICATED_PAGE_LIMIT = 1; +const AUTHENTICATED_PAGE_LIMIT = 10; + +function isUsableCommitName(name, login, knownLogins) { + const normalized = name.toLowerCase(); + if (!name || normalized === String(login ?? '').toLowerCase() || knownLogins.has(normalized)) { + return false; + } + if (/\d/.test(name) || /^@/.test(name) || /\[bot\]|copilot|dependabot|chatgpt|openai|gemini|cursor|codex/i.test(name)) { + return false; + } + if (name.includes('-') && !name.split('-').every((segment) => /^[A-Z][a-z]+$/.test(segment))) { + return false; + } + return true; +} + +export async function fetchCommitAuthors({ + owner, repo, defaultBranch, initialCommits = [], knownGithubLogins = [], + warnings, authToken = '', cleanString, normalizeAuthor, normalizeAuthors, + addWarning, fetchOptionalJson, +}) { + const knownLogins = new Set(knownGithubLogins.map((login) => cleanString(login).toLowerCase())); + const authorNames = []; + let commits = Array.isArray(initialCommits) ? initialCommits : []; + let page = 1; + const maxPages = authToken ? AUTHENTICATED_PAGE_LIMIT : UNAUTHENTICATED_PAGE_LIMIT; + + while (true) { + for (const commit of commits) { + const name = cleanString(commit?.commit?.author?.name ?? ''); + const login = commit?.author?.login; + if (commit?.author?.type && commit.author.type !== 'User') continue; + if (isUsableCommitName(name, login, knownLogins)) authorNames.push(name); + } + + if (commits.length < COMMIT_PAGE_SIZE) break; + if (page >= maxPages) { + addWarning(warnings, 'commit-authors', 'commit-author-scan-limited', + `Scanned the first ${page * COMMIT_PAGE_SIZE} commits for contributor author names.`, + { owner, repo, scannedPages: page, scannedCommits: page * COMMIT_PAGE_SIZE }); + break; + } + + page += 1; + commits = await fetchOptionalJson( + buildGithubCommitListApiUrl(owner, repo, defaultBranch, COMMIT_PAGE_SIZE, page), + buildGithubRequestConfig({ + authToken, + source: 'commit-authors', + label: `commit authors page ${page}`, + onWarning: (source, code, message, details = {}) => addWarning(warnings, source, code, message, details), + }), + ) || []; + if (!Array.isArray(commits) || commits.length === 0) break; + } + + return dedupeAuthors(normalizeAuthors(authorNames.map((name) => normalizeAuthor({ name })))); +} \ No newline at end of file diff --git a/src/services/githubImporterContributors.js b/src/services/githubImporterContributors.js index 3c63d18..27929c0 100644 --- a/src/services/githubImporterContributors.js +++ b/src/services/githubImporterContributors.js @@ -375,5 +375,6 @@ export async function fetchContributorAuthors({ return { fallbackAuthors: dedupeAuthors(normalizeAuthors(fallbackAuthors)), lookupAuthors: dedupeAuthors(normalizeAuthors(lookupAuthors)), + githubLogins: contributors.map((contributor) => cleanString(contributor?.login ?? '').toLowerCase()).filter(Boolean), }; } diff --git a/tests/services/commitAuthors.test.js b/tests/services/commitAuthors.test.js new file mode 100644 index 0000000..1819cbf --- /dev/null +++ b/tests/services/commitAuthors.test.js @@ -0,0 +1,41 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; + +import { fetchCommitAuthors } from '../../src/services/githubImporterCommitAuthors.js'; +import { cleanString, normalizeAuthor, normalizeAuthors } from '../../src/services/githubImporterUtils.js'; + +function commit(name, login = '') { + return { commit: { author: { name } }, author: login ? { login, type: 'User' } : null }; +} + +test('authenticated scans continue to page two for human author names', async () => { + const urls = []; + const warnings = []; + const result = await fetchCommitAuthors({ + owner: 'test-owner', repo: 'test-repo', defaultBranch: 'feature/new', authToken: 'token', + initialCommits: Array.from({ length: 100 }, () => commit('known-login')), + knownGithubLogins: ['known-login'], warnings, + cleanString, normalizeAuthor, normalizeAuthors, + addWarning: (items, source, code) => items.push({ source, code }), + fetchOptionalJson: async (url) => { + urls.push(url); + return [commit('Ada Lovelace'), commit('bot-123'), commit('name-handle'), commit('Alice Example', 'Alice Example')]; + }, + }); + assert.equal(urls.length, 1); + assert.equal(urls[0].endsWith('commits?per_page=100&sha=feature%2Fnew&page=2'), true); + assert.deepEqual(result.map((author) => `${author.givenNames} ${author.familyNames}`), ['Ada Lovelace']); + assert.equal(warnings.length, 0); +}); + +test('unauthenticated scans stop after the first 100 commits', async () => { + const warnings = []; + const result = await fetchCommitAuthors({ + owner: 'test-owner', repo: 'test-repo', initialCommits: Array.from({ length: 100 }, () => commit('Dana Example')), + warnings, cleanString, normalizeAuthor, normalizeAuthors, + addWarning: (items, source, code) => items.push({ source, code }), + fetchOptionalJson: async () => { throw new Error('Should not request another page'); }, + }); + assert.equal(result.length, 1); + assert.equal(warnings.some((warning) => warning.code === 'commit-author-scan-limited'), true); +}); \ No newline at end of file diff --git a/tests/services/githubImporter.test.js b/tests/services/githubImporter.test.js index 55df6c5..7dfaeaf 100644 --- a/tests/services/githubImporter.test.js +++ b/tests/services/githubImporter.test.js @@ -982,7 +982,7 @@ test('importGithubMetadata ignores username-like contributors when no real profi return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1061,7 +1061,7 @@ test('importGithubMetadata excludes AI bot co-authors and contributor accounts w return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1224,7 +1224,7 @@ test('importGithubMetadata ignores GitHub usernames in co-author names and prefe return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1304,7 +1304,7 @@ test('importGithubMetadata prefers commit co-author names over username fallback return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1383,7 +1383,7 @@ test('importGithubMetadata includes co-authored contributor names from commit me return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1461,7 +1461,7 @@ test('importGithubMetadata includes co-authored contributor names from recent hi return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: { @@ -1540,7 +1540,7 @@ test('importGithubMetadata includes co-authored contributor names when a release }); } - if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=10&sha=main')) { + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return Response.json([ { commit: {