Repository navigation
Conversation
Signed-off-by: Robert Washbourne <rawashbourne@gmail.com>
Bugbot is paused — your team has an unpaid invoiceBugbot is paused because your team has an unpaid invoice. Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically. (requestId: 3b92f487-9d40-4f4d-b249-bf80f09c5b4a) |
Try out this PRQuick install: pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@44c296f712d87e1e5eba3d58257b337cbd9e3385Recommended with virtual environment (using uv): uv venv --python 3.12 && source .venv/bin/activate
uv pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@44c296f712d87e1e5eba3d58257b337cbd9e3385Last updated for commit: |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09760f19aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return row | ||
|
|
||
| def test_arrow_roundtrip_preserves_valid_trace(self) -> None: | ||
| from datasets import Dataset |
There was a problem hiding this comment.
Move the gated datasets import to module scope
On Windows ARM, datasets is intentionally absent (pyproject.toml:30-34), and the unit-suite gate only ignores modules whose top-level imports are detected (tests/harness/optional_deps.py:89-120). Because this import is inside the test, test_semianalysis_cc_traces_weka.py is still collected and then fails here with ModuleNotFoundError; move it (and the local ValidationError import below) to module scope so the existing scanner can skip the module and the test follows the repository's top-level-import convention.
AGENTS.md reference: AGENTS.md:L105-L105
Useful? React with 👍 / 👎.
|
@rawsh -- thanks for contributing a fix. I'll review this soon. In the mean time, I'm also exploring what it will take to bring the InferenceX runs up to using |
tests/harness/optional_deps.py skips test modules by their top-level imports, so a function-level datasets import fails instead of skipping where datasets is unavailable (Windows ARM). Signed-off-by: Robert Washbourne <rawashbourne@gmail.com>
|
Ported upstream: ai-dynamo#1647. Also moved the datasets imports to module scope. |
Loading
semianalysisai/cc-traces-weka-062126through HF datasets fills absent union fields withnull, which then failsWekaTracevalidation. This drops thosenullkeys for fields the target model doesn't define; non-null extra fields still fail. Adds tests.Note
Low Risk
Scoped to the HF Weka loader validation path with tests; file loading and rejection of malformed rows stay strict.
Overview
Fixes HuggingFace loading of SemiAnalysis Weka corpora (e.g.
semianalysisai/cc-traces-weka-062126) where Arrow union encoding injectsnullkeys for fields that belong to other request variants, causing strictWekaTracePydantic validation to fail.The HF loader now runs each row through
_strip_hf_null_paddingbeforeWekaTrace.model_validate, recursively trimming absent union fields per discriminated request type (n,s,subagent) while keeping strict behavior for non-null unknown keys and null required fields. The file-based--input-filepath is unchanged.Unit tests simulate an Arrow
Datasetroundtrip and assert padding is stripped without mutating the source row; docs note the HF-only relaxation.Reviewed by Cursor Bugbot for commit 44c296f. Bugbot is set up for automated code reviews on this repo. Configure here.