Skip to content

Switch online data sampling from GitHub archive to GitHub search api - #54

Merged
ashleyzhang01 merged 19 commits into
mainfrom
switch-online-data-sampling-from-github-archive-to-github-search-api
Aug 26, 2026
Merged

Switch online data sampling from GitHub archive to GitHub search api#54
ashleyzhang01 merged 19 commits into
mainfrom
switch-online-data-sampling-from-github-archive-to-github-search-api

Conversation

@ashleyzhang01

Copy link
Copy Markdown
Contributor
  • PR discovery and volume now use GitHub’s Search API instead of GitHub Archive. Archive was missing a large share of GitHub App review events, so public volume and sampled PRs were both undercounted.
  • Fix a bug with copilot comment attribution by adding alias
  • Additional optional dashboard filter for PRs reviewed by only one of the code review bots we track (many PRs use multiple review bots)

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 migrates online PR discovery and volume counting to GitHub Search API, adds Copilot actor aliases, and introduces solo-bot dashboard filtering.

Changes:

  • Adds Search API discovery, pagination, sampling, volume counting, and CLI controls.
  • Updates actor normalization, ETL processing, and documentation.
  • Adds solo-bot API/dashboard support and updates offline benchmark artifacts.

Reviewed changes

Copilot reviewed 25 out of 27 changed files in this pull request and generated 8 comments.

Show a summary per file
File Summary Final review status
online/README.md Updates ETL documentation. No final comments.
online/etl/tests/test_quality_signals.py Tests Copilot alias recognition. No final comments.
online/etl/tests/test_discover_search_api.py Tests Search API discovery. No final comments.
online/etl/tests/test_assemble.py Tests Search API assembly. No final comments.
online/etl/tests/test_actor_normalization.py Tests actor aliases and engagement detection. No final comments.
online/etl/README.md Documents Search API volumes and processing modes. Nit (3 votes): Documentation and implementation use different volume-date semantics; align them or document creation-date volumes.
online/etl/pipeline/quality.py Recognizes normalized bot aliases. No final comments.
online/etl/pipeline/label.py Adds per-day labeling limits. No final comments.
online/etl/pipeline/discover.py Implements Search API discovery and sampling. Critical (4 votes): merged_at is used as bot_reviewed_at; persist the matched review’s submitted_at or use a separately named merge-time field. Moderate (3 votes): Default Search ordering biases sampling when a day exceeds 1,000 results; partition windows or use an unbiased strategy.
online/etl/pipeline/assemble.py Supports Search API metadata. No final comments.
online/etl/pipeline/analyze.py Integrates updated analysis controls. No final comments.
online/etl/pipeline/actors.py Adds GitHub actor aliases. No final comments.
online/etl/main.py Adds source and processing CLI options. Moderate (3 votes): --min-pr-number is ignored by the Search API branch; reject it or implement an equivalent filter. Moderate (2 votes): The scheduled job still uses BigQuery/Archive discovery; propagate the source choice.
online/etl/db/repository.py Adds bounded and per-day query dispatch. Critical (4 votes): The per-day branch can override sweep/until and ignore the overall limit. Critical (3 votes): Label-side per-day processing drops limit and until, allowing unbounded and out-of-range labeling.
online/etl/db/queries.py Defines processing query variants. Moderate (3 votes): SQLite cast stripping prevents calendar-day grouping, so per-day caps do not work correctly; use DATE(...) or dialect-specific SQL in all variants.
online/etl/config.py Updates ETL configuration. No final comments.
online/api_service/static/index.html Adds the solo-bot dashboard filter. No final comments.
online/api_service/src/tests.rs Tests solo-bot filtering. No final comments.
online/api_service/src/model.rs Adds solo-bot filter state. No final comments.
online/api_service/src/handlers.rs Parses the new filter. No final comments.
online/api_service/src/db.rs Computes bot participation metadata. No final comments.
online/api_service/src/compute.rs Applies solo-bot filtering. No final comments.
online/api_service/README.md Documents API behavior and filters. No final comments.
offline/code_review_benchmark/step_speed_analysis.py Adds benchmark timing analysis. No final comments.
offline/analysis/benchmark_dashboard.py Updates dashboard generation. No final comments.
offline/analysis/benchmark_dashboard.json Embeds updated benchmark data. No final comments.
Suppressed comments (16)

online/api_service/src/db.rs:157

  • The new solo-status computation is not covered by the added Rust test: test_require_solo_bot manually sets PrRecord.is_solo_bot and therefore cannot catch grouping/identity mistakes here. Add a build_snapshot test with two rows for the same (repo_name, pr_number) and different chatbot_ids (and a single-row case) to verify the field is populated correctly before filtering.
    // Solo vs multi-bot: a GitHub PR is solo if exactly one original chatbot_id
    // scored it. Display merge (MERGE_INTO) is ignored here — two Qodo accounts
    // on the same PR still confuse the judge.
    let mut bots_per_github_pr: HashMap<(&str, i32), HashSet<i32>> = HashMap::new();
    for row in &rows {
        bots_per_github_pr
            .entry((row.repo_name.as_str(), row.pr_number))
            .or_default()
            .insert(row.chatbot_id);
    }

online/etl/README.md:146

  • The default command now uses Search API, but this section leaves the setup instructions saying a GCP/BigQuery grant is required (and the introduction still says discovery is via BigQuery). Following the documented default workflow therefore imposes an unnecessary dependency; make BigQuery requirements conditional on --source bq and document the Search API token as the default requirement.
# All chatbots, last 7 days (uses GitHub Search API by default)
uv run python main.py volumes --all --days-back 7

online/etl/db/queries.py:416

  • The label-side query has the same SQLite behavior: its ::date cast is stripped by DBAdapter, so rows are partitioned by full timestamp instead of calendar date. This lets labeling exceed the advertised per-day cap. Use the same dialect-neutral date expression here.
                   PARTITION BY p.bot_reviewed_at::date

online/etl/pipeline/actors.py:9

  • This alias makes copilot-swe-agent count as Copilot during attribution, but the Search API path still queries only copilot-pull-request-reviewer[bot] for the Copilot chatbot (pipeline/volumes.py:24-26, consumed by discovery and volumes). PRs reviewed only by the SWE agent will therefore be absent from both sampled PRs and volume counts even though this code now classifies them as Copilot activity. Please make the search mapping support both slugs (or remove this alias if SWE-agent is not intended to be tracked).
    "copilot-swe-agent": "copilot",

online/etl/pipeline/discover.py:713

  • Copilot is normalized to both copilot-pull-request-reviewer and copilot-swe-agent, but Search discovery maps Copilot to only the reviewer account. PRs reviewed only by the SWE-agent account are therefore never discovered by the new default source. Query all Copilot review-account aliases and merge/deduplicate the results before insertion.
    search_username = SEARCH_API_USERNAME_MAP.get(chatbot_username, chatbot_username)

online/etl/pipeline/discover.py:649

  • A transient count failure is converted into an empty window and the command continues as if that day had no PRs. This silently undercounts discovery and leaves no failed date to retry on the next run; propagate the transient error or record the window as failed so the job cannot report success with missing data.
    if isinstance(count, SearchError):
        logger.warning(f"Search API error for {search_username} in {merged_range}: {count}")
        return []

online/etl/pipeline/discover.py:613

  • If a page request fails transiently, pagination stops and the partial page set is sampled and inserted as a successful day. That produces a silently incomplete sample even though the count query reported more results. Do not treat a partial pagination result as complete; retry or fail the window instead.
        result = await _search_api_fetch_page(pool, query, page)
        if isinstance(result, SearchError):
            logger.warning(f"Stopping pagination at page {page} due to {result}")
            break

online/etl/pipeline/discover.py:735

  • The CLI documents --max-prs-per-day as the sample cap, but this fetch limit is hard-capped at 1,000. For any requested cap above 1,000, _sample_prs can never return the requested amount even though _fetch_window has bisection support. Either validate the option's maximum or let large requests use the bisection path.
            fetch_limit = min(max_prs_per_day * 2, _SEARCH_MAX_RESULTS)

online/etl/pipeline/discover.py:688

  • The Search API does not request a random order, so prs is in GitHub's relevance/default order. This cap keeps the first 10 PRs from every repository, meaning PRs after that position can never be sampled and repeated runs are biased toward the same PRs despite the function's random-sampling contract. Randomly select up to max_per_repo items per repository before applying the daily cap (or use reservoir sampling).
    for pr in prs:
        repo = pr["repo_name"]
        repo_counts[repo] = repo_counts.get(repo, 0) + 1
        if repo_counts[repo] <= max_per_repo:
            capped.append(pr)

online/etl/pipeline/discover.py:14

  • The new itertools import is separated from and placed after the other standard-library imports. The ETL Ruff configuration enables isort with force-sort-within-sections, so this produces an import-order lint error; move it into the sorted standard-library block.
import itertools

online/etl/tests/test_discover_search_api.py:351

  • This local is assigned but never read. The ETL Ruff configuration enables the F checks, so it triggers F841 and prevents lint from passing; the mocked method is replaced below and this assignment can be removed.
        original_get = c.get

online/etl/tests/test_discover_search_api.py:171

  • url and kwargs are unused in this new mock signature. Ruff's ARG checks are enabled for the ETL tests, so this definition reports unused arguments; use underscore-prefixed names (the signature remains compatible with AsyncClient.get).
    async def mock_get(url: str, params: dict | None = None, **kwargs) -> MagicMock:  # type: ignore[type-arg]

online/etl/tests/test_discover_search_api.py:201

  • url and kwargs are unused in this new mock signature. Ruff's ARG checks are enabled for the ETL tests, so this definition reports unused arguments; use underscore-prefixed names (the signature remains compatible with AsyncClient.get).
    async def mock_get(url: str, params: dict | None = None, **kwargs) -> MagicMock:  # type: ignore[type-arg]

online/etl/tests/test_discover_search_api.py:244

  • url and kwargs are unused in this new mock signature. Ruff's ARG checks are enabled for the ETL tests, so this definition reports unused arguments; use underscore-prefixed names (the signature remains compatible with AsyncClient.get).
    async def mock_get(url: str, params: dict | None = None, **kwargs) -> MagicMock:  # type: ignore[type-arg]

online/etl/tests/test_discover_search_api.py:283

  • url and kwargs are unused in this new mock signature. Ruff's ARG checks are enabled for the ETL tests, so this definition reports unused arguments; use underscore-prefixed names (the signature remains compatible with AsyncClient.get).
    async def mock_get(url: str, params: dict | None = None, **kwargs) -> MagicMock:  # type: ignore[type-arg]

online/etl/tests/test_discover_search_api.py:338

  • url and kwargs are unused in this new mock signature. Ruff's ARG checks are enabled for the ETL tests, so this definition reports unused arguments; use underscore-prefixed names (the signature remains compatible with AsyncClient.get).
    async def mock_get(url: str, params: dict | None = None, **kwargs) -> MagicMock:  # type: ignore[type-arg]

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

Comment thread online/etl/README.md
```

Queries `githubarchive.day.*` for all PR-related events (reviews, review comments, issue comments on PRs), assigns each unique PR to the first day the bot touched it, and upserts daily counts into the `pr_volumes` table. Each PR is counted exactly once so summing daily counts gives the true unique total — this ensures total PRs >= sampled PRs holds in the leaderboard. Days with no activity for a bot are zero-filled by the API service.
Default source is GitHub Search API (`--source search-api`), which counts PRs where the bot left a review. Each PR is counted exactly once on the earliest day the bot touched it. Days with no activity are zero-filled by the API service.
Comment thread online/etl/db/queries.py Outdated
Comment on lines +162 to +164
PARTITION BY p.bot_reviewed_at::date
ORDER BY random()
) AS rn
Comment on lines +160 to +164
if max_per_day is not None and since:
if chatbot_id is not None:
return await self.db.fetchall(
q.GET_ASSEMBLED_PRS_NOT_ANALYZED_PER_DAY, (chatbot_id, since, max_per_day)
)
Comment on lines +339 to +343
if max_per_day is not None and since:
if chatbot_id is not None:
return await self.db.fetchall(
q.GET_ANALYZED_NOT_LABELED_PER_DAY, (chatbot_id, since, max_per_day)
)
Comment thread online/etl/main.py
Comment on lines +355 to +359
if use_search_api:
await discover_prs_search_api_batch(
cfg,
db,
usernames,
Comment thread online/etl/main.py
Comment on lines +153 to +156
"--source",
choices=["search-api", "bq"],
default="search-api",
help="Discovery source: 'search-api' (default, GitHub Search API) or 'bq' (BigQuery/GH Archive)",
Comment thread online/etl/pipeline/discover.py Outdated
Comment on lines +523 to +525
# merged_at is the best proxy for bot_reviewed_at from search results;
# the bot must have reviewed before the merge happened
"bot_reviewed_at": pr_data.get("merged_at"),
Comment on lines +735 to +738
fetch_limit = min(max_prs_per_day * 2, _SEARCH_MAX_RESULTS)
raw_items = await _fetch_window(
pool, search_username, window_start, window_end,
max_items=fetch_limit,
@ashleyzhang01
ashleyzhang01 merged commit b6583aa into main Aug 26, 2026
2 checks passed
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.

3 participants