Skip to content

Fix various signal issues - #831

Open
inferno-chromium wants to merge 1 commit into
ossf:mainfrom
inferno-chromium:fix/collector-signal-accuracy
Open

Fix various signal issues#831
inferno-chromium wants to merge 1 commit into
ossf:mainfrom
inferno-chromium:fix/collector-signal-accuracy

Conversation

@inferno-chromium

@inferno-chromium inferno-chromium commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Four defects in how repository signals are gathered:

Issue counts read zero for every repository. GitHub moved the REST issues endpoint to cursor based pagination, so it returns a "next" link and no "last" link. FetchIssueCount derived its total by requesting one issue per page and reading the page count out of that "last" link; with the link gone the count parsed as 0, and because a "next" page was still reported the function took its "more than one page" branch and returned that 0. Any repository with more than a single issue in the window reported none.

That zeroed three of the ten default signals, 19% of the weight: updated_issues_count, closed_issues_count, and issue_comment_frequency, the last because it is skipped when the updated count is zero. Against the live API kubernetes/kubernetes reported 0/0/0 and scored 0.776, where the README documents 5395/3062/5.5 and 0.99107. Simulated across the 35 benchmark projects the correction moves every one of them, by a median of +0.105 and as much as +0.190, and it moves them unevenly, so it changed ranks and not just values.

The counts now come from the search index. FetchIssueCounts returns the updated and closed totals from one request with four aliased searches, one per combination of issue or pull request and updated or closed. The caller computes a single cutoff and passes it to both the counts and the comment count they are divided by, so every part of comment frequency covers the same window; deriving it separately let the numerator and denominator differ. The cutoff is a full RFC3339 timestamp rather than a date, which would move the boundary by up to a day. Search returns an exact total regardless of size, so MaxIssuesLimit and its cap are removed.

Two properties of search make this easy to break invisibly, so both are asserted against the generated query rather than only its variables: a query missing an is:issue or is:pull-request qualifier returns a count of zero instead of an error, and every search connection carries first: 1. That is not required to read issueCount, a scalar on the connection rather than a traversal of its nodes, but it states the boundary explicitly.

Projects that track bugs elsewhere reported zero issues. torvalds/linux, git/git, openssh/openssh-portable, FFmpeg/FFmpeg and sqlite/sqlite disable the GitHub issue tracker. Recording 0 asserts that nobody reports bugs against the Linux kernel. The fields are now left unset, which scoring skips, using hasIssuesEnabled from the existing query, so it costs no extra call and saves three per affected repository. Correcting this alone, with the scoring untouched, raises the median infrastructure score on the benchmark from 0.635 to 0.759. It is a trade: a repository with the tracker off still receives pull requests, and those are no longer counted for it.

A transient server error was treated as a permanent one. Every 5xx from the comment endpoint was classified as "too many results" and replaced with a constant of 2.0, which sits below the frequency measured on quiet repositories, so the fallback ranked the busiest projects below inactive ones. The classification was also simply wrong: kubernetes/kubernetes returned a 5xx on one run and the real count on the next. Transient failures are now retried, the cause is preserved, the field is left unset when it still fails, and the loss is logged at warning rather than debug, because a run that quietly drops a signal for many repositories should be visible.

Time and frequency were miscomputed. updated_since read the head commit's author date, which Git preserves across rebases and mail-based patch workflows: madler/zlib's head is authored 2026-04-05 and committed 2026-06-01, so the project looked two months more neglected than it was, and patch-based projects were penalised systematically. TimeDelta took an absolute difference, so a commit dated in the future produced a large positive age; Git timestamps come from the committer's machine and are not validated. commit_frequency divided a 365 day count by 52 rather than 52.14.

The same page count idiom counts contributors and finds a repository's first commit, and both now handle a missing "last" link rather than reading it as an empty result. They are deliberately inconsistent: contributors fails the repository, being the heaviest weighted signal, where a silent zero is worse than a retryable error; the comment count leaves one field unset; and the first commit lookup returns the caller's existing upper bound, which is what it is documented to return when no earlier time can be established.

internal/collector/github had no tests. There are now tests that a disabled tracker issues no requests at all, that the counts map to the right fields, that comment frequency is comments per updated item, and that it is left unknown rather than zero when it cannot be determined.

@inferno-chromium
inferno-chromium marked this pull request as draft August 10, 2026 04:42
@inferno-chromium
inferno-chromium marked this pull request as ready for review August 10, 2026 04:42
Four defects in how repository signals are gathered, each of which produced
a number that looked ordinary and was wrong.

Issue counts read zero for every repository. GitHub moved the REST issues
endpoint to cursor based pagination, so it returns a "next" link and no
"last" link. FetchIssueCount derived its total by requesting one issue per
page and reading the page count out of that "last" link; with the link gone
the count parsed as 0, and because a "next" page was still reported the
function took its "more than one page" branch and returned that 0. Any
repository with more than a single issue in the window reported none.

That zeroed three of the ten default signals, 19% of the weight:
updated_issues_count, closed_issues_count, and issue_comment_frequency, the
last because it is skipped when the updated count is zero. Against the live
API kubernetes/kubernetes reported 0/0/0 and scored 0.776, where the README
documents 5395/3062/5.5 and 0.99107. Simulated across the 35 benchmark
projects the correction moves every one of them, by a median of +0.105 and
as much as +0.190, and it moves them unevenly, so it changed ranks and not
just values.

The counts now come from the search index. FetchIssueCounts returns the
updated and closed totals from one request with four aliased searches, one
per combination of issue or pull request and updated or closed. The caller
computes a single cutoff and passes it to both the counts and the comment
count they are divided by, so every part of comment frequency covers the
same window; deriving it separately let the numerator and denominator differ.
The cutoff is a full RFC3339 timestamp rather than a date, which would move
the boundary by up to a day. Search returns an exact total regardless of
size, so MaxIssuesLimit and its cap are removed.

Two properties of search make this easy to break invisibly, so both are
asserted against the generated query rather than only its variables: a query
missing an is:issue or is:pull-request qualifier returns a count of zero
instead of an error, and every search connection carries first: 1. That is
not required to read issueCount, a scalar on the connection rather than a
traversal of its nodes, but it states the boundary explicitly.

Projects that track bugs elsewhere reported zero issues. torvalds/linux,
git/git, openssh/openssh-portable, FFmpeg/FFmpeg and sqlite/sqlite disable
the GitHub issue tracker. Recording 0 asserts that nobody reports bugs
against the Linux kernel. The fields are now left unset, which scoring
skips, using hasIssuesEnabled from the existing query, so it costs no extra
call and saves three per affected repository. Correcting this alone, with
the scoring untouched, raises the median infrastructure score on the
benchmark from 0.635 to 0.759. It is a trade: a repository with the tracker
off still receives pull requests, and those are no longer counted for it.

A transient server error was treated as a permanent one. Every 5xx from the
comment endpoint was classified as "too many results" and replaced with a
constant of 2.0, which sits below the frequency measured on quiet
repositories, so the fallback ranked the busiest projects below inactive
ones. The classification was also simply wrong: kubernetes/kubernetes
returned a 5xx on one run and the real count on the next. Transient failures
are now retried, the cause is preserved, the field is left unset when it
still fails, and the loss is logged at warning rather than debug, because a
run that quietly drops a signal for many repositories should be visible.

Time and frequency were miscomputed. updated_since read the head commit's
author date, which Git preserves across rebases and mail-based patch
workflows: madler/zlib's head is authored 2026-04-05 and committed
2026-06-01, so the project looked two months more neglected than it was, and
patch-based projects were penalised systematically. TimeDelta took an
absolute difference, so a commit dated in the future produced a large
positive age; Git timestamps come from the committer's machine and are not
validated. commit_frequency divided a 365 day count by 52 rather than 52.14.

The same page count idiom counts contributors and finds a repository's first
commit, and both now handle a missing "last" link rather than reading it as
an empty result. They are deliberately inconsistent: contributors fails the
repository, being the heaviest weighted signal, where a silent zero is worse
than a retryable error; the comment count leaves one field unset; and the
first commit lookup returns the caller's existing upper bound, which is what
it is documented to return when no earlier time can be established.

internal/collector/github had no tests. There are now tests that a disabled
tracker issues no requests at all, that the counts map to the right fields,
that comment frequency is comments per updated item, and that it is left
unknown rather than zero when it cannot be determined.

Signed-off-by: Abhishek Arya <inferno@chromium.org>
@inferno-chromium
inferno-chromium force-pushed the fix/collector-signal-accuracy branch from 306f5f8 to 2754a29 Compare August 10, 2026 04:54
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.

1 participant