From 7e355734394a7f6906f968aa3005db1c766016e6 Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Sat, 26 Sep 2026 22:59:58 -0400 Subject: [PATCH 1/5] 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 6f4c607..2af68ba 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,12 @@ that branch until the first workflow deployment succeeds. 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 3ce8ddf..ba62576 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 83fbd12..effcdf9 100644 --- a/tests/services/githubImporter.test.js +++ b/tests/services/githubImporter.test.js @@ -996,7 +996,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: { @@ -1075,7 +1075,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: { @@ -1238,7 +1238,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: { @@ -1318,7 +1318,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: { @@ -1397,7 +1397,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: { @@ -1475,7 +1475,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: { @@ -1554,7 +1554,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: { From e1e4ab71cfc53956141b834db658f7232d6832a8 Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Sun, 8 Nov 2026 15:44:34 -0500 Subject: [PATCH 2/5] feat: enhance commit author filtering to exclude automated contributors and improve test coverage --- src/services/githubImporter.js | 2 - src/services/githubImporterCommitAuthors.js | 15 +++-- src/services/githubImporterContributors.js | 2 +- tests/services/commitAuthors.test.js | 28 ++++++++ tests/services/githubImporter.test.js | 73 +++++++++------------ 5 files changed, 71 insertions(+), 49 deletions(-) diff --git a/src/services/githubImporter.js b/src/services/githubImporter.js index ba62576..933cd1f 100644 --- a/src/services/githubImporter.js +++ b/src/services/githubImporter.js @@ -732,7 +732,6 @@ export async function importGithubMetadata(repoUrl, options = {}) { const contributorLookupAuthors = dedupeAuthors([ ...coAuthorAuthors, ...contributorResult.lookupAuthors.filter(Boolean), - ...commitAuthors, ]); addRateLimitHintIfNeeded(warnings, authToken); @@ -824,4 +823,3 @@ export async function importGithubMetadata(repoUrl, options = {}) { return { metadata, warnings, errors, review, healthScan, comparisons }; } - diff --git a/src/services/githubImporterCommitAuthors.js b/src/services/githubImporterCommitAuthors.js index 6411e04..99a713c 100644 --- a/src/services/githubImporterCommitAuthors.js +++ b/src/services/githubImporterCommitAuthors.js @@ -1,19 +1,26 @@ import { buildGithubCommitListApiUrl, buildGithubRequestConfig } from './githubApi.js'; import { dedupeAuthors } from './githubImporterAuthors.js'; +import { isAutomatedContributorIdentity } from './githubImporterContributors.js'; const COMMIT_PAGE_SIZE = 100; const UNAUTHENTICATED_PAGE_LIMIT = 1; const AUTHENTICATED_PAGE_LIMIT = 10; -function isUsableCommitName(name, login, knownLogins) { +function isUsableCommitName(name, login, knownLogins, cleanString) { 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)) { + if ( + /\d/.test(name) + || /[_@]/.test(name) + || /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(name) + || isAutomatedContributorIdentity(name, cleanString) + || isAutomatedContributorIdentity(login, cleanString) + ) { return false; } - if (name.includes('-') && !name.split('-').every((segment) => /^[A-Z][a-z]+$/.test(segment))) { + if (!/\s/.test(name) && name.includes('-') && !name.split('-').every((segment) => /^[A-Z][a-z]+$/.test(segment))) { return false; } return true; @@ -35,7 +42,7 @@ export async function fetchCommitAuthors({ 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 (isUsableCommitName(name, login, knownLogins, cleanString)) authorNames.push(name); } if (commits.length < COMMIT_PAGE_SIZE) break; diff --git a/src/services/githubImporterContributors.js b/src/services/githubImporterContributors.js index 27929c0..d5cba16 100644 --- a/src/services/githubImporterContributors.js +++ b/src/services/githubImporterContributors.js @@ -10,7 +10,7 @@ const TOP_CONTRIBUTOR_FALLBACK_LIMIT = 4; const MAX_CONTRIBUTOR_FALLBACK_LIMIT = 20; const GITHUB_PAGE_SIZE = 100; -function isAutomatedContributorIdentity(value, cleanString) { +export function isAutomatedContributorIdentity(value, cleanString) { const text = cleanString(value ?? '').trim(); if (!text) { return false; diff --git a/tests/services/commitAuthors.test.js b/tests/services/commitAuthors.test.js index 1819cbf..68ce638 100644 --- a/tests/services/commitAuthors.test.js +++ b/tests/services/commitAuthors.test.js @@ -38,4 +38,32 @@ test('unauthenticated scans stop after the first 100 commits', async () => { }); assert.equal(result.length, 1); assert.equal(warnings.some((warning) => warning.code === 'commit-author-scan-limited'), true); +}); + +test('commit history excludes automated identities and aliases but accepts full hyphenated names', async () => { + const result = await fetchCommitAuthors({ + owner: 'test-owner', + repo: 'test-repo', + initialCommits: [ + commit('Jean-Pierre Dupont'), + commit('Mary Smith-Jones'), + commit('Claude Code'), + commit('GitHub Actions'), + commit('GitHub Actions', 'actions-bot'), + commit('Claude Fable'), + commit('alice_dev'), + commit('dev@example.com'), + ], + warnings: [], + cleanString, + normalizeAuthor, + normalizeAuthors, + addWarning: () => {}, + fetchOptionalJson: async () => [], + }); + + assert.deepEqual(result.map((author) => `${author.givenNames} ${author.familyNames}`), [ + 'Jean Pierre Dupont', + 'Mary Smith Jones', + ]); }); \ No newline at end of file diff --git a/tests/services/githubImporter.test.js b/tests/services/githubImporter.test.js index effcdf9..82d6603 100644 --- a/tests/services/githubImporter.test.js +++ b/tests/services/githubImporter.test.js @@ -1614,8 +1614,15 @@ test('importGithubMetadata includes co-authored contributor names when a release } }); -test('importGithubMetadata orders imported authors by contributor rank', async () => { +test('importGithubMetadata does not reorder metadata authors based on history-only commits', async () => { const originalFetch = globalThis.fetch; + const jsonResponse = (data) => ({ + ok: true, + status: 200, + statusText: 'OK', + headers: { get: () => null }, + text: async () => JSON.stringify(data), + }); const citationText = `cff-version: 1.2.0\ntitle: "OpenCite"\nversion: "1.0.0"\ndate-released: "2025-01-02"\nrepository-code: "https://github.com/test-owner/test-repo"\nauthors:\n - family-names: "Doe"\n given-names: "Jane"\n - family-names: "Smith"\n given-names: "John"\n`; const citationBase64 = Buffer.from(citationText, 'utf8').toString('base64'); @@ -1624,7 +1631,7 @@ test('importGithubMetadata orders imported authors by contributor rank', async ( const value = String(url); if (value.endsWith('/repos/test-owner/test-repo')) { - return Response.json({ + return jsonResponse({ name: 'test-repo', html_url: 'https://github.com/test-owner/test-repo', default_branch: 'main', @@ -1639,55 +1646,35 @@ test('importGithubMetadata orders imported authors by contributor rank', async ( } if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=1&sha=main')) { - return Response.json([{ commit: { committer: { date: '2025-01-02T00:00:00Z' } } }]); + return jsonResponse([{ commit: { committer: { date: '2025-01-02T00:00:00Z' } } }]); + } + + if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { + return jsonResponse([ + { commit: { author: { name: 'John Smith' } }, author: null }, + ]); } if (value.endsWith('/repos/test-owner/test-repo/branches/main')) { - return Response.json({ name: 'main' }); + return jsonResponse({ name: 'main' }); } if (value.includes('/repos/test-owner/test-repo/contents/CITATION.cff?ref=main')) { - return Response.json({ encoding: 'base64', content: citationBase64 }); + return jsonResponse({ encoding: 'base64', content: citationBase64 }); } if (value.includes('/repos/test-owner/test-repo/contents/')) { - return new Response(JSON.stringify({ message: 'Not Found' }), { status: 404, headers: { 'Content-Type': 'application/json' } }); + return { + ok: false, + status: 404, + statusText: 'Not Found', + headers: { get: () => null }, + text: async () => JSON.stringify({ message: 'Not Found' }), + }; } if (value.includes('/repos/test-owner/test-repo/contributors?')) { - // John appears first => highest contributor rank. - return Response.json([ - { login: 'johnsmith', type: 'User' }, - { login: 'janedoe', type: 'User' }, - ]); - } - - if (value.endsWith('/users/janedoe')) { - return Response.json({ - login: 'janedoe', - type: 'User', - name: 'Jane Doe', - company: 'Imageomics', - html_url: 'https://github.com/janedoe', - }); - } - - if (value.endsWith('/users/johnsmith')) { - return Response.json({ - login: 'johnsmith', - type: 'User', - name: 'John Smith', - company: 'Imageomics', - html_url: 'https://github.com/johnsmith', - }); - } - - if (value.endsWith('/users/janedoe/social_accounts') || value.endsWith('/users/johnsmith/social_accounts')) { - return Response.json([]); - } - - if (value === 'https://github.com/janedoe' || value === 'https://github.com/johnsmith') { - return new Response('', { status: 200, headers: { 'Content-Type': 'text/html' } }); + return jsonResponse([]); } throw new Error(`Unexpected fetch URL: ${value}`); @@ -1698,10 +1685,12 @@ test('importGithubMetadata orders imported authors by contributor rank', async ( contributorFallbackLimit: 5, }); - assert.equal(result.errors.length, 0); + assert.equal(result.errors.length, 0, JSON.stringify(result.errors)); assert.equal(result.metadata.authors.length >= 2, true); - assert.equal(result.metadata.authors[0].givenNames, 'John'); - assert.equal(result.metadata.authors[0].familyNames, 'Smith'); + assert.equal(result.metadata.authors[0].givenNames, 'Jane'); + assert.equal(result.metadata.authors[0].familyNames, 'Doe'); + assert.equal(result.metadata.authors[1].givenNames, 'John'); + assert.equal(result.metadata.authors[1].familyNames, 'Smith'); } finally { globalThis.fetch = originalFetch; } From d64c40ed75413d66bb7e304809f1e683bd49316c Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Sun, 8 Nov 2026 17:09:28 -0500 Subject: [PATCH 3/5] feat: improve author discovery in commit history and enhance related tests --- README.md | 5 +-- src/services/githubImporterCommitAuthors.js | 12 ++++--- tests/services/commitAuthors.test.js | 36 +++++++++++++++++++++ tests/services/githubImporter.test.js | 11 ++++--- 4 files changed, 53 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2af68ba..022c9ad 100644 --- a/README.md +++ b/README.md @@ -126,8 +126,9 @@ that branch until the first workflow deployment succeeds. 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. +and GitHub handles found in contributor profiles or anywhere in the scanned +commits rather than using them as citation names. Verify imported authors before +exporting citation metadata. ## Validation Behavior diff --git a/src/services/githubImporterCommitAuthors.js b/src/services/githubImporterCommitAuthors.js index 99a713c..92155b9 100644 --- a/src/services/githubImporterCommitAuthors.js +++ b/src/services/githubImporterCommitAuthors.js @@ -32,7 +32,7 @@ export async function fetchCommitAuthors({ addWarning, fetchOptionalJson, }) { const knownLogins = new Set(knownGithubLogins.map((login) => cleanString(login).toLowerCase())); - const authorNames = []; + const discoveredAuthors = []; let commits = Array.isArray(initialCommits) ? initialCommits : []; let page = 1; const maxPages = authToken ? AUTHENTICATED_PAGE_LIMIT : UNAUTHENTICATED_PAGE_LIMIT; @@ -40,9 +40,10 @@ export async function fetchCommitAuthors({ while (true) { for (const commit of commits) { const name = cleanString(commit?.commit?.author?.name ?? ''); - const login = commit?.author?.login; + const login = cleanString(commit?.author?.login ?? ''); + if (login) knownLogins.add(login.toLowerCase()); if (commit?.author?.type && commit.author.type !== 'User') continue; - if (isUsableCommitName(name, login, knownLogins, cleanString)) authorNames.push(name); + discoveredAuthors.push({ name, login }); } if (commits.length < COMMIT_PAGE_SIZE) break; @@ -66,5 +67,8 @@ export async function fetchCommitAuthors({ if (!Array.isArray(commits) || commits.length === 0) break; } - return dedupeAuthors(normalizeAuthors(authorNames.map((name) => normalizeAuthor({ name })))); + const authorNames = discoveredAuthors + .filter(({ name, login }) => isUsableCommitName(name, login, knownLogins, cleanString)) + .map(({ name }) => normalizeAuthor({ name })); + return dedupeAuthors(normalizeAuthors(authorNames)); } \ No newline at end of file diff --git a/tests/services/commitAuthors.test.js b/tests/services/commitAuthors.test.js index 68ce638..eb4758f 100644 --- a/tests/services/commitAuthors.test.js +++ b/tests/services/commitAuthors.test.js @@ -28,6 +28,42 @@ test('authenticated scans continue to page two for human author names', async () assert.equal(warnings.length, 0); }); +test('commit names matching logins on any scanned page are excluded', async () => { + const initialCommits = [ + commit('Jane-Doe'), + commit('Anna-Maria'), + commit('Mary Smith-Jones'), + ...Array.from({ length: 97 }, () => commit('Repeated Person')), + ]; + const result = await fetchCommitAuthors({ + owner: 'test-owner', + repo: 'test-repo', + defaultBranch: 'main', + authToken: 'token', + initialCommits, + knownGithubLogins: [], + warnings: [], + cleanString, + normalizeAuthor, + normalizeAuthors, + addWarning: () => {}, + fetchOptionalJson: async () => [ + commit('Real Author', 'jane-doe'), + commit('Another Contributor', 'ANNA-MARIA'), + commit('Jean-Luc'), + commit('Full Name', 'jean-luc'), + ], + }); + + assert.deepEqual(result.map((author) => `${author.givenNames} ${author.familyNames}`), [ + 'Mary Smith Jones', + 'Repeated Person', + 'Real Author', + 'Another Contributor', + 'Full Name', + ]); +}); + test('unauthenticated scans stop after the first 100 commits', async () => { const warnings = []; const result = await fetchCommitAuthors({ diff --git a/tests/services/githubImporter.test.js b/tests/services/githubImporter.test.js index 82d6603..c866b04 100644 --- a/tests/services/githubImporter.test.js +++ b/tests/services/githubImporter.test.js @@ -1652,6 +1652,8 @@ test('importGithubMetadata does not reorder metadata authors based on history-on if (value.endsWith('/repos/test-owner/test-repo/commits?per_page=100&sha=main')) { return jsonResponse([ { commit: { author: { name: 'John Smith' } }, author: null }, + { commit: { author: { name: 'Ada Lovelace' } }, author: null }, + { commit: { author: { name: 'Ada Lovelace' } }, author: null }, ]); } @@ -1686,11 +1688,10 @@ test('importGithubMetadata does not reorder metadata authors based on history-on }); assert.equal(result.errors.length, 0, JSON.stringify(result.errors)); - assert.equal(result.metadata.authors.length >= 2, true); - assert.equal(result.metadata.authors[0].givenNames, 'Jane'); - assert.equal(result.metadata.authors[0].familyNames, 'Doe'); - assert.equal(result.metadata.authors[1].givenNames, 'John'); - assert.equal(result.metadata.authors[1].familyNames, 'Smith'); + assert.deepEqual( + result.metadata.authors.map(({ givenNames, familyNames }) => `${givenNames} ${familyNames}`), + ['Jane Doe', 'John Smith', 'Ada Lovelace'], + ); } finally { globalThis.fetch = originalFetch; } From 5de14fcf5f23d847783baf4752e2c6692bfb392d Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Sun, 8 Nov 2026 17:34:32 -0500 Subject: [PATCH 4/5] rebasing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From 714e139f9aa84c6bce3a6cbc26ca7d786c797cbf Mon Sep 17 00:00:00 2001 From: beanbean9339 Date: Mon, 9 Nov 2026 10:46:21 -0500 Subject: [PATCH 5/5] fix: update download button text to reflect all files in ZIP --- src/App.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App.jsx b/src/App.jsx index 992b93b..d0bd6af 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -1431,7 +1431,7 @@ export default function App() { disabled={isZipping} > {isZipping &&