diff --git a/.agents/handoffs/issue-1571-handoff.md b/.agents/handoffs/issue-1571-handoff.md index c8a4c319a..cc06eb6a1 100644 --- a/.agents/handoffs/issue-1571-handoff.md +++ b/.agents/handoffs/issue-1571-handoff.md @@ -15,9 +15,9 @@ - Branch: `issue/1571-flaky-gpu-test-retry` - Base: `origin/main` at `9d909f5f0 [test]: remove dead and duplicate tests (-489 lines) (#1556)` - Handoff path: `.agents/handoffs/issue-1571-handoff.md` -- Current stage: Stage 1 - Deep Dive And Plan -- Implementation begun: no. Stage 1 is analysis/planning only. -- Last updated: 2026-07-10T08:40:26Z +- Current stage: Stage 2 - Implement The User-Directed Fix +- Implementation begun: yes. User approved the recommended central Modal pytest rerun approach. +- Last updated: 2026-07-10T10:23:10Z ## Stage 0 Notes @@ -52,6 +52,11 @@ - #1389 `[ci] Modal: enable Flash Attention 3 on H100 perf tests with build cache`, ready-for-review, not draft. Modal/perf infrastructure, not pytest rerun support. - #1494 and #1183 are attention-related but not focused on CI retry behavior. - No PR draft status was changed. +- Stage 2 re-check on 2026-07-10T10:10:42Z: + - Verified `gh` identity as `macthecadillac`. + - Issue #1571 remains OPEN with no comments and no assignees. + - Focused open PR search for issue number/title/`pytest-rerunfailures`/`--only-rerun`/reruns found no direct implementation of this issue. + - No PR draft status was changed. ## Investigation Log @@ -159,7 +164,52 @@ - Recommended approach: central pytest-rerunfailures support in Modal orchestrators with a transient-only regex, plus docs. - Awaiting user guidance before Stage 2 implementation. +## Selected Approach + +- User approved the Stage 1 recommendation on 2026-07-10. +- Implement Approach B: + - Add `pytest-rerunfailures` to the test extra. + - Add one Modal-local helper for retry args and transient regex. + - Apply it centrally to `fastvideo/tests/modal/pr_test.py` and `fastvideo/tests/modal/ssim_test.py`. + - Add focused static contract coverage. + - Update CI/testing docs. + +## Stage 2 Implementation Log + +- Implemented central Modal pytest retry wiring: + - Added `fastvideo/tests/modal/pytest_retry.py` with shared `pytest-rerunfailures` args, retry count `2`, delay `10s`, and a conservative transient-only regex. + - Updated `fastvideo/tests/modal/pr_test.py` to compute `PYTEST_ADDOPTS` from the cloned repo's helper after checkout, so Modal does not need to mount helper files beside the entrypoint. + - Updated `fastvideo/tests/modal/ssim_test.py` to append the same retry args to each SSIM subprocess pytest command. + - Added `pytest-rerunfailures` to the `test` extra in `pyproject.toml`. + - Added static contract coverage in `fastvideo/tests/contract/test_ci_pytest_reruns.py`. + - Updated `docs/contributing/ci_architecture.md` and `docs/contributing/testing.md` with durable retry policy documentation. +- Local cleanup: + - `uv run --with modal ... --help` was accidentally started from the repo without `--no-project`; it began resolving FastVideo dependencies and created a small `.venv`. + - Stopped that process and removed the generated `.venv`. + +## Validation Log + +- Do not run project tests locally rule followed; validation was run on Modal L40S through `/tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py`. +- Failed validation attempt 1: + - Command: + `uv run --no-project --with modal python -m modal run /tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py --install-extra test --no-build-kernel --apply-local-patch --patch-paths "pyproject.toml,fastvideo/tests/modal/pr_test.py,fastvideo/tests/modal/ssim_test.py,fastvideo/tests/modal/pytest_retry.py,fastvideo/tests/contract/test_ci_pytest_reruns.py,docs/contributing/ci_architecture.md,docs/contributing/testing.md" --command "pytest fastvideo/tests/contract/test_ci_test_collection.py fastvideo/tests/contract/test_ci_pytest_reruns.py -q"` + - Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-r6oqddOSxQlgIduaTgx2Jq + - Result: failed before tests. `uv pip install -e '.[test]'` attempted to build `fastvideo-kernel==0.3.2` from sdist and failed because `cutlass/cutlass.h` was missing from the package build context. + - Assessment: unrelated to this CI retry patch; the targeted static tests do not require installing FastVideo or building kernels. +- Failed validation attempt 2: + - Command used `--install-extra none` and `uv pip install pytest` in the remote command. + - Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-njpA3gOITr1UJ4QO3DdqD6 + - Result: failed before tests with `/bin/bash: line 1: uv: command not found` because `--install-extra none` skips environment sourcing that places `uv` on PATH. + - Assessment: command setup issue only. +- Passing validation: + - Command: + `uv run --no-project --with modal python -m modal run /tmp/fastvideo-worktrees/interleavethinker-launcher/fastvideo/tests/modal/launch_l40s_job.py --install-extra none --no-build-kernel --apply-local-patch --patch-paths "pyproject.toml,fastvideo/tests/modal/pr_test.py,fastvideo/tests/modal/ssim_test.py,fastvideo/tests/modal/pytest_retry.py,fastvideo/tests/contract/test_ci_pytest_reruns.py,docs/contributing/ci_architecture.md,docs/contributing/testing.md" --command "source $HOME/.local/bin/env 2>/dev/null || true; source /opt/venv/bin/activate 2>/dev/null || true; uv pip install pytest && pytest fastvideo/tests/contract/test_ci_test_collection.py fastvideo/tests/contract/test_ci_pytest_reruns.py -q"` + - Modal URL: https://modal.com/apps/hao-ai-lab/main/ap-Z7mqFKXntYOTP64dNof6m8 + - Result: passed, `6 passed in 0.04s`. + - Modal return summary: local patch applied, `install_extra=none`, `build_kernel=False`, commit `732f9df86c5c67b6f55a7dd1e7400e1b78d9e69a`. + ## Next Steps -- Commit and push this handoff-only Stage 1 state. -- Present Stage 1 report to the user and ask whether to implement the recommended approach. +- Stage and commit signed implementation changes. +- Push branch. +- Proceed to Stage 3 review/adjudication loop before presenting the draft PR message. diff --git a/docs/contributing/ci_architecture.md b/docs/contributing/ci_architecture.md index 12829aeb4..6a9d0fdf4 100644 --- a/docs/contributing/ci_architecture.md +++ b/docs/contributing/ci_architecture.md @@ -125,6 +125,18 @@ it), while a GitHub outage or a >25 min wait lets it run anyway (fail open). See [Performance Benchmarks](performance_benchmarks.md) for the performance lane's thresholds, rolling baseline, artifacts, and reseeding process. +### Modal Pytest Reruns + +Modal pytest lanes install `pytest-rerunfailures` through the `test` extra. +`fastvideo/tests/modal/pr_test.py` applies the shared retry policy through +`PYTEST_ADDOPTS`, and `fastvideo/tests/modal/ssim_test.py` appends the same +arguments to each SSIM subprocess. + +The policy reruns an individual pytest failure twice with a short delay, but +only when the failure text matches the transient infrastructure regex in +`fastvideo/tests/modal/pytest_retry.py`. Plain assertion and numerical parity +failures are not matched by that policy and should fail without rerun. + ## Slash Commands Slash commands are handled by `.github/workflows/ci-slash-commands.yml`. @@ -239,7 +251,9 @@ All Buildkite test jobs go through `.buildkite/scripts/pr_test.sh`, which: 3. Passes Buildkite metadata into the Modal container. 4. Runs the selected test command from `fastvideo/tests/modal/pr_test.py` or `fastvideo/tests/modal/ssim_test.py`. -5. Uploads performance artifacts for `TEST_TYPE=performance`. +5. Applies the shared transient-only pytest rerun policy to Modal pytest + commands. +6. Uploads performance artifacts for `TEST_TYPE=performance`. If you add a new CI test category: diff --git a/docs/contributing/testing.md b/docs/contributing/testing.md index 7cd5c1060..f71ff928d 100644 --- a/docs/contributing/testing.md +++ b/docs/contributing/testing.md @@ -219,6 +219,12 @@ instances. The main files are: For exact tier membership, path filters, slash commands, and aggregate statuses, see [CI/CD Architecture](ci_architecture.md). +Modal pytest commands use `pytest-rerunfailures` with the shared transient-only +policy in `fastvideo/tests/modal/pytest_retry.py`. The retry layer reruns +individual pytest failures for infrastructure-style errors such as connection, +Modal worker, NCCL, or CUDA initialization failures. Deterministic assertion +and numerical parity failures should still fail immediately. + ### Adding A New CI Test Category If a new test does not fit an existing lane: diff --git a/fastvideo/tests/contract/test_ci_pytest_reruns.py b/fastvideo/tests/contract/test_ci_pytest_reruns.py new file mode 100644 index 000000000..e13bb6ad7 --- /dev/null +++ b/fastvideo/tests/contract/test_ci_pytest_reruns.py @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Static checks for Modal pytest rerun wiring. + +Pure text analysis: no fastvideo imports, no GPU, no torch. +""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +PYPROJECT = REPO_ROOT / "pyproject.toml" +MODAL_ROOT = REPO_ROOT / "fastvideo" / "tests" / "modal" +PYTEST_RETRY = MODAL_ROOT / "pytest_retry.py" +PR_TEST = MODAL_ROOT / "pr_test.py" +SSIM_TEST = MODAL_ROOT / "ssim_test.py" + + +def test_pytest_rerunfailures_is_in_test_extra(): + assert '"pytest-rerunfailures"' in PYPROJECT.read_text() + + +def test_modal_entrypoints_share_pytest_retry_policy(): + retry_text = PYTEST_RETRY.read_text() + pr_text = PR_TEST.read_text() + ssim_text = SSIM_TEST.read_text() + + assert "TRANSIENT_FAILURE_REGEX" in retry_text + assert "--only-rerun" in retry_text + assert "build_pytest_addopts" in pr_text + assert "PYTEST_ADDOPTS" in pr_text + assert "_build_pytest_rerun_args()" in ssim_text + + +def test_retry_policy_excludes_plain_assertion_failures(): + retry_text = PYTEST_RETRY.read_text() + + assert "AssertionError" not in retry_text + assert "assert " not in retry_text diff --git a/fastvideo/tests/modal/pr_test.py b/fastvideo/tests/modal/pr_test.py index b972458cf..eb8f82c83 100644 --- a/fastvideo/tests/modal/pr_test.py +++ b/fastvideo/tests/modal/pr_test.py @@ -1,5 +1,6 @@ import os import re +import shlex import shutil import subprocess import sys @@ -194,6 +195,24 @@ def run_test(pytest_command: str): run_test_command(pytest_command, build_kernel=True) +def _pytest_retry_setup_command() -> str: + addopts_statement = ( + "import os, sys; " + "sys.path.insert(0, 'fastvideo/tests/modal'); " + "from pytest_retry import build_pytest_addopts; " + "print(build_pytest_addopts(os.environ.get('PYTEST_ADDOPTS', '')))" + ) + describe_statement = ( + "import sys; " + "sys.path.insert(0, 'fastvideo/tests/modal'); " + "from pytest_retry import describe_pytest_reruns; " + "print(describe_pytest_reruns())" + ) + addopts_command = f"python -c {shlex.quote(addopts_statement)}" + describe_command = f"python -c {shlex.quote(describe_statement)}" + return f'export PYTEST_ADDOPTS="$({addopts_command})" &&\n {describe_command} &&' + + def run_test_command(test_command: str, build_kernel: bool, install_command: str = 'uv pip install -e ".[test]"'): @@ -232,6 +251,7 @@ def run_test_command(test_command: str, source /opt/venv/bin/activate && cd /FastVideo && {install_clause} + {_pytest_retry_setup_command()} {build_kernel_command} {test_command} """ diff --git a/fastvideo/tests/modal/pytest_retry.py b/fastvideo/tests/modal/pytest_retry.py new file mode 100644 index 000000000..63f1d08f3 --- /dev/null +++ b/fastvideo/tests/modal/pytest_retry.py @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: Apache-2.0 +"""Shared pytest rerun policy for Modal CI entrypoints.""" + +from __future__ import annotations + +import shlex + +PYTEST_RERUNS = 2 +PYTEST_RERUNS_DELAY_SECONDS = 10 + +TRANSIENT_FAILURE_PATTERNS = ( + r"connection (?:reset|aborted|refused|timed out)", + r"read timed out", + r"remote end closed connection", + r"temporarily unavailable", + r"(?:HTTP|status code) 5\d\d", + r"502 Bad Gateway", + r"503 Service Unavailable", + r"504 Gateway Timeout", + r"NCCL.*(?:timeout|connection|remote error|unhandled system error|system error)", + r"CUDA error: (?:unknown error|initialization error|operation not permitted|operation not supported)", + r"Modal.*(?:preempt|interrupt|worker|container|timeout)", +) +TRANSIENT_FAILURE_REGEX = "(?i)(" + "|".join(TRANSIENT_FAILURE_PATTERNS) + ")" + + +def build_pytest_rerun_args() -> list[str]: + return [ + "--reruns", + str(PYTEST_RERUNS), + "--reruns-delay", + str(PYTEST_RERUNS_DELAY_SECONDS), + "--only-rerun", + TRANSIENT_FAILURE_REGEX, + ] + + +def build_pytest_addopts(existing_addopts: str = "") -> str: + retry_addopts = shlex.join(build_pytest_rerun_args()) + existing_addopts = existing_addopts.strip() + if existing_addopts: + return f"{existing_addopts} {retry_addopts}" + return retry_addopts + + +def describe_pytest_reruns() -> str: + return ( + "Pytest transient reruns enabled: " + f"{PYTEST_RERUNS} reruns, {PYTEST_RERUNS_DELAY_SECONDS}s delay, " + f"only-rerun={TRANSIENT_FAILURE_REGEX!r}" + ) diff --git a/fastvideo/tests/modal/ssim_test.py b/fastvideo/tests/modal/ssim_test.py index 1b904421c..a5664470d 100644 --- a/fastvideo/tests/modal/ssim_test.py +++ b/fastvideo/tests/modal/ssim_test.py @@ -680,7 +680,7 @@ def _build_pytest_extra_args( ssim_bootstrap_mode: bool, pytest_k: str, ) -> list[str]: - args = [] + args = _build_pytest_rerun_args() if ssim_full_quality: args.append("--ssim-full-quality") if ssim_reference_repo.strip(): @@ -694,6 +694,21 @@ def _build_pytest_extra_args( return args +def _build_pytest_rerun_args() -> list[str]: + helper_paths = [ + os.path.dirname(os.path.abspath(__file__)), + os.path.join("/FastVideo", "fastvideo", "tests", "modal"), + ] + for helper_path in helper_paths: + if helper_path not in sys.path: + sys.path.insert(0, helper_path) + + from pytest_retry import build_pytest_rerun_args, describe_pytest_reruns # noqa: E402 + + print(describe_pytest_reruns()) + return build_pytest_rerun_args() + + def _schedule_ssim_tasks( repo_root: str, tasks: list[SSIMTask], diff --git a/pyproject.toml b/pyproject.toml index 53c351bbb..d95350a71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -136,6 +136,7 @@ test = [ "av", "pytorch-msssim>=1.0.0", "pytest", + "pytest-rerunfailures", "pandas", "plotly", ]