Skip to content

fix: improve contributor fallback and author visibility - #64

Open
beanbean9339 wants to merge 13 commits into
mainfrom
fix/contributor-fallback-limit
Open

beanbean9339 wants to merge 13 commits into
mainfrom
fix/contributor-fallback-limit

Conversation

@beanbean9339

Copy link
Copy Markdown
Collaborator

Summary

Closes #56

This pull request improves GitHub contributor fallback handling by ensuring automated accounts are excluded from citation authors and eligible human contributors are identified more reliably.

Changes

  • Improved automated account detection to identify additional AI-assisted coding accounts and bots, including Copilot, Gemini, ChatGPT, OpenAI, and Cursor.
  • Updated contributor fallback logic to include eligible human contributors rather than limiting the results based on the raw GitHub contributor list.
  • Added support for anonymous commit authors through the GitHub Contributors API.
  • Improved commit-history scanning to identify contributors who may not appear in the initial contributor results.
  • Added pagination support for commit-history requests when an authentication token is available.
  • Updated author visibility in the metadata form so all authors are displayed by default.
  • Improved author filtering to exclude automated accounts and unusable GitHub username-like values.

Testing

  • Added and updated regression tests for:
    • Automated contributor detection and filtering.
    • Contributor fallback behavior.
    • Anonymous commit authors.
    • Commit-history author discovery.
    • Contributor deduplication and filtering.

Related Issue

Closes #56

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.

🟡 Changes recommended

Unresolved fixture, contributor ordering, fallback-limit, and example-naming issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Improves GitHub contributor fallback, filtering, anonymous author discovery, and author visibility.

Changes:

  • Expands automated-account filtering and contributor discovery.
  • Adds pagination and historical commit-author scanning.
  • Updates author visibility, tests, examples, and documentation.
File summaries
File Summary Findings
tests/services/githubImporter.test.js Adds regression coverage Fixtures require updates for the new request shape.
src/services/githubImporterContributors.js Filters and discovers contributors Moderate (3 votes): fallback limits are no longer honored.
src/services/githubImporter.js Integrates historical authors Critical (1 vote): existing fixtures fail with per_page=100 URLs.
Moderate (1 vote): commit-history authors override established contributor ordering.
src/services/githubApi.js Adds pagination and anonymous-author parameters No review findings.
src/services/github.examples.js Updates importer examples Nit (1 vote): rename the misleading exampleCustomContributorLimit export.
src/components/MetadataForm.jsx Shows all authors by default No review findings.
src/App.jsx Removes fallback-limit configuration No review findings.
README.md Documents contributor behavior No review findings.
Review details

Suppressed comments (2)

src/services/github.examples.js:84

  • This example is still exported as exampleCustomContributorLimit, but the updated body no longer demonstrates or accepts a custom limit. The name now misleads callers about what the example does; rename the example (and any references) to describe contributor fallback authors instead.
 * Example: Contributor fallback authors
 * Includes all eligible human contributors when repository metadata has no authors.
 */
export async function exampleCustomContributorLimit() {
  try {
    const repoUrl = 'https://github.com/imageomics/OpenCite';

    const { metadata, warnings } = await importGithubMetadata(repoUrl);

src/services/githubImporter.js:743

  • orderAuthorsByContributorRank ranks authors by their position in contributorLookupAuthors; prepending commitAuthors here lets commit-history order override the existing co-author/contributor ranking. For a repository whose recent commit author is not the highest-ranked contributor, this changes the documented contributor ordering. Append the new commit-history entries after the existing lookup list so the prior ordering remains authoritative.
  const contributorLookupAuthors = dedupeAuthors([
    ...commitAuthors,
    ...coAuthorAuthors,
    ...contributorResult.lookupAuthors.filter(Boolean),
  • Files reviewed: 8/8 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/services/githubImporter.js
Comment thread src/services/githubImporterContributors.js Outdated

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.

🟡 Changes recommended

Address stale test mocks, unbounded API scans, and overly broad display-name filtering.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/services/githubImporterContributors.js:226

  • When an authentication token is present, maxPages is null, so a long repository history is scanned one 100-commit request at a time until the end. A single import can therefore issue thousands of requests, consume the token's rate limit, and remain slow or fail; authenticated scans still need a finite safety cap and a warning when that cap is reached.

src/services/githubImporter.js:553

  • This request-size change leaves several importer test fixtures matching only commits?per_page=1&sha=main (for example tests/services/githubImporter.test.js:759, 839, 898, 1087, 1671, 1817, and 1910). The importer now requests per_page=100, so those tests fall through to Unexpected fetch URL before exercising the behavior; update all remaining mocks to accept the new URL (or make the mock branch accept both).
    buildGithubCommitListApiUrl(owner, repo, defaultBranch, 100),

src/services/githubImporterContributors.js:385

  • This applies the username heuristic to a verified GitHub profile display name, so legitimate human names such as Anne-Marie or McDonald are discarded merely because they contain a hyphen or camel case, even when they differ from the login. Use the profile/login relationship to reject unusable handles without filtering valid display-name punctuation and casing.
      if (profile?.name && !matchesGithubLoginName(profile.name, login, cleanString) && !isLikelyGithubUsername(profile.name, cleanString)) {
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/services/githubImporterContributors.js Outdated

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.

Copilot review overview

🔵 Needs a closer look

Address unbounded contributor requests, ordering and name-filtering regressions, and strengthen anonymous-author test coverage.

Review effort: Lite
Findings: 1 High severity · 2 Medium severity

Open (3)

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.

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues affect rate limits, filtering correctness, safety limits, and test coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
Resolved since last review (3)

Comment thread src/services/githubImporterContributors.js
Comment thread README.md Outdated

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.

Copilot review overview

🟡 Changes recommended

Unresolved contributor fallback safety and API compatibility issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Retain a maximum cap for contributor fallback requests

src/​services/​githubImporterContributors.js:218

This removes the previous upper safety bound, so a caller can pass a very large finite contributorFallbackLimit and make the importer paginate through and concurrently profile an unbounded number of GitHub accounts. That can exhaust API rate limits and memory; keep a documented maximum (for example the default 50) when normalizing the option.

This issue also appears on line 341 of the same file.

Comment thread src/services/github.examples.js

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.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings affect fallback limits and username filtering.

Review effort: Lite
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Apply fallback limit after profile eligibility filtering

src/​services/​githubImporterContributors.js:199

This cap is applied before the profile requests below determine whether a contributor has a usable human name or is automated by its profile name. Such entries still count toward contributors.length, so a repository with an unusable/late-detected account in the first page can stop pagination and return fewer than the requested number of eligible fallback authors even when later pages contain valid people. Apply the limit after profile eligibility is resolved, or continue fetching pages until the cap is reached by actual fallback authors.

Low severity Assert anonymous contributors are imported within the limit

tests/​services/​githubImporter.test.js:116

Although this fixture includes an anonymous contributor, the fallback limit is 2 and the expected result only contains Alice and Bob, so Dana Anonymous is never asserted as an imported author. A regression that drops the anonymous-record branch would still pass; add an assertion (or a focused case) where an anonymous record falls within the fallback limit.

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.

Copilot review overview

🟡 Changes recommended

A regression test fails, and fallback-limit and unauthenticated API rate-limit issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Preserve null sentinel for unlimited contributor fallback

src/​services/​githubImporterContributors.js:248

This changes the established null/empty option behavior: resolveContributorFallbackLimit previously returned null to disable the cap, and the fallback code below still contains an unlimited branch for that sentinel. Converting these values to 50 silently caps callers that request an unlimited fallback and makes that branch unreachable; preserve the sentinel through candidate selection/slicing or explicitly remove and document the public behavior change.

Comment thread tests/services/githubImporter.test.js Outdated
Comment thread src/services/githubImporterContributors.js

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.

Copilot review overview

🔵 Needs a closer look

Moderate findings remain around author caps, non-user filtering, and the compatibility example behavior.

Review effort: Lite
Findings: None

Resolved since last review (2)

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.

Fix contributor fallback limit when automated accounts are present

2 participants