Add scheduled adapter ingestion workflow#184
Conversation
This script runs all Every Eval Ever adapters, manages their execution, and uploads results to Hugging Face.
Updated the repository ID and modified the upload process to use upload_large_folder. Added checks for input requirements and improved error handling during adapter processing.
Sanitize filenames to avoid issues with Hugging Face Hub by replacing invalid characters and handling names starting with 'x00'.
Updating files
Added 'env' parameter to get_dynamic_args function to allow passing environment variables for subprocess execution. Updated subprocess calls to use the provided environment.
Refactor adapter staleness check and input requirement parsing. Removed unused header checks and simplified logic for determining if an adapter is stale.
Refactor run_adapters.py by removing unused imports, simplifying functions, and updating the download_hf_json function to remove revision parameter. Additionally, streamline adapter processing and improve header checks.
tommasocerruti
left a comment
There was a problem hiding this comment.
@DeepLumiere Thank you for the great work! 🚀 Could you please:
- Retitle this PR to something like “Add scheduled adapter ingestion workflow”;
- Mark it as a draft while adapter coverage and failure handling are still wip;
- Add a living description with the goal, design, and an adapter status table that you can update as support improves;
- Split the unrelated changes (e.g., ARC adapter, helper change, ...) into separate PRs. Unless there is a strong reason, let's keep it focused on the scheduled adapter runner;
- Address the inline comments I left.
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version-file: "pyproject.toml" |
There was a problem hiding this comment.
Would it be safer to pin Python 3.12 here? Since the constraint in pyproject.toml is >=3.12, the latest run selected Python 3.14.6, which appears to have caused the datasets/dill failures in the SWE adapters.
|
|
||
| return False | ||
|
|
||
| def get_input_requirement(adapter_path: Path): |
There was a problem hiding this comment.
I’m a little concerned about discovering adapter arguments by parsing their source code with regexes. This can easily miss multiline constants or slightly different CLI conventions. Would an explicit adapter config be simpler and more reliable? An alternative idea could be a common CLI contract for every adapter.
There was a problem hiding this comment.
I’m a little concerned about discovering adapter arguments by parsing their source code with regexes. This can easily miss multiline constants or slightly different CLI conventions. Would an explicit adapter config be simpler and more reliable? An alternative idea could be a common CLI contract for every adapter.
A contract I feel that was discussed as a base class of adapter so that future work doesn't need major adaptation with Leshem Choshen and Harsha Nelaturu. I'll check it out whether something like that already exists, else try to make it.
| DATA_DIR.mkdir(exist_ok=True) | ||
|
|
||
| # Download existing state | ||
| stats = download_hf_json("adapter_stats.json", {}) |
There was a problem hiding this comment.
Is this path intentional? We write the file as data/adapter_stats.json below, but read adapter_stats.json from the repository root here. Also, this reads from the default branch while the upload creates a PR, so I’m not sure the state from one run will be available to the next one.
There was a problem hiding this comment.
As of now decision making as per previous one is to be decided, weekly or daily what to do with heavy ones and all, any suggestions?
| "time_s": elapsed, | ||
| "size_mb": dir_size_mb, | ||
| "last_success_ts": time.time(), | ||
| "last_failed": False |
There was a problem hiding this comment.
Could we only mark an adapter successful once validation has produced at least one valid file and no invalid files? At the moment, missing validator output or an incorrect output directory results in 0 valid, 0 failed, but the adapter is still recorded as successful.
| folder_path=".", | ||
| repo_type=REPO_TYPE, | ||
| allow_patterns=["data/**"], | ||
| create_pr=True |
There was a problem hiding this comment.
How are subsequent runs expected to update the same Hugging Face PR? Would it make sense to keep the returned PR revision after the first upload and target that revision on later runs? It would also be useful to log the returned commit/PR URL so we can verify what was updated.
| ) | ||
| print("Upload complete!") | ||
| except Exception as e: | ||
| print(f"Upload failed: {e}") |
There was a problem hiding this comment.
Should this failure be re-raised or should the script exit nonzero at the end? Right now an upload failure (as well as the adapter failures collected above) can still leave the GitHub Actions run green, which makes the scheduled job look healthier than it is.
# Conflicts: # utils/scripts/run_adapters.py
Refactor run_adapters.py for improved readability and functionality. Changes include updating function signatures, enhancing error handling, and optimizing file operations.
feat: add Mercor evaluation adapter (evaleval#199)
No description provided.