Skip to content

Separate benchmark pipeline into dedicated top-level directory - #417

Open
oussamahansal wants to merge 2 commits into
mainfrom
split-benchmarks-from-integration-tests
Open

Separate benchmark pipeline into dedicated top-level directory#417
oussamahansal wants to merge 2 commits into
mainfrom
split-benchmarks-from-integration-tests

Conversation

@oussamahansal

@oussamahansal oussamahansal commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Description

Separate benchmark pipeline into dedicated top-level benchmarks/ directory

Changes

  • Move benchmark scripts (benchmark_build.py, benchmark_query.py, benchmark_evaluate.py, benchmark_extract.py) from integration-tests/test-scripts/graphrag_toolkit_tests/ to benchmarks/scripts/
  • Move benchmark utilities (run_evaluation.py, retriever_factory.py, token_tracker.py, etc.) from integration-tests/test-scripts/graphrag_toolkit_tests/benchmark_utils/ to benchmarks/utils/
  • Move benchmark dataset suite files to benchmarks/datasets/
  • Move benchmark unit tests to benchmarks/tests/
  • Copy shared test infrastructure (integration_test_base.py, integration_test_handler.py) into benchmarks/scripts/ with canonical-source headers
  • Delete legacy benchmark-tests/ directory
  • Add benchmarks/env.template with benchmark-specific variables only
  • Add benchmarks/requirements.txt for benchmark-specific dependencies
  • Update build-tests.sh to resolve suite files from benchmarks/datasets/ fallback path
  • Update test_suite.py routing logic to handle benchmarks.* module imports
  • Rewrite imports across all relocated files to use new benchmarks.* package paths

Problem

Benchmark pipeline code was interleaved with integration test infrastructure, making it difficult to run benchmarks independently, onboard new datasets, or understand which files belonged to which workflow.

Related issue (if any): #

Testing

  • Unit tests added/updated
  • Integration tests added (as appropriate)
  • Existing tests pass (pytest)
  • Tested manually (describe below)

Checklist

  • Code follows existing style and conventions
  • License headers present on new files
  • Documentation updated (if applicable)
  • No breaking changes (or clearly documented)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mykola-pereyma mykola-pereyma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions:

  1. Copy drift riskintegration_test_base.py and integration_test_handler.py are copied into benchmarks/scripts/ with a canonical-source header. If the canonical source in integration-tests/ changes, the benchmark copy goes stale silently. Would a symlink, a shared internal package, or a direct import from the integration-tests path work instead?

  2. Ordering with PR #416 — Both PRs modify the same benchmark files (retriever_factory.py, benchmark_query.py). Whichever merges second will have conflicts. Which should land first, and should the other be rebased on top?

  3. How to run benchmarks from new location — The README should document the new invocation path. How does test_suite.py routing work with the new benchmarks.* module paths? A contributor looking at benchmarks/ for the first time would not know how to run them.

@oussamahansal

Copy link
Copy Markdown
Collaborator Author

Few questions:

  1. Copy drift riskintegration_test_base.py and integration_test_handler.py are copied into benchmarks/scripts/ with a canonical-source header. If the canonical source in integration-tests/ changes, the benchmark copy goes stale silently. Would a symlink, a shared internal package, or a direct import from the integration-tests path work instead?
  2. Ordering with PR [benchmark]: Add multi-retriever benchmarking, Nova 2 Lite support, and PGA bio/stat split #416 — Both PRs modify the same benchmark files (retriever_factory.py, benchmark_query.py). Whichever merges second will have conflicts. Which should land first, and should the other be rebased on top?
  3. How to run benchmarks from new location — The README should document the new invocation path. How does test_suite.py routing work with the new benchmarks.* module paths? A contributor looking at benchmarks/ for the first time would not know how to run them.
  1. The integration_test_base.py and integration_test_handler.py files are copied into benchmarks/scripts/ (not symlinked) because SageMaker notebook deployments package and upload the directory — symlinks would break.
  2. this refactoring can go first and ill fix any conflicts after.
  3. the readme should include all the new paths and how to run with the new refactoring.. let me know if it's not clear

@mykola-pereyma mykola-pereyma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

2 participants