perf: reduce pytest parallelism from -n auto to -n 8#1013
Conversation
On machines with 32 logical CPUs, -n auto spawns 32 workers which causes crashes (resource contention, SQLite locks) and is actually slower due to GIL contention and context switching overhead. Cap at 8 workers for local development. CI keeps -n auto since GitHub Actions runners have 2-4 cores where auto is appropriate.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (1)pyproject.toml📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2026-04-02T16:35:04.612ZApplied to files:
📚 Learning: 2026-03-15T18:28:13.207ZApplied to files:
📚 Learning: 2026-03-31T14:17:24.182ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
🔇 Additional comments (1)
WalkthroughThis pull request standardizes pytest parallelism across documentation and configuration by replacing 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
Updates contributor-facing documentation to use a fixed local pytest-xdist worker count (-n 8) instead of -n auto, aiming to avoid over-parallelization on high-core developer machines while keeping CI behavior unchanged.
Changes:
- Switched documented local pytest commands from
-n autoto-n 8across contributor docs and internal checklists. - Clarified in
CLAUDE.mdthat local runs should use-n 8while CI continues using-n auto. - Updated “verification / pre-PR” skill steps to match the new local test invocation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/guides/contributing.md | Updates pytest command examples and the “Parallelism” rule to -n 8. |
| docs/getting_started.md | Updates quick-start pytest commands to -n 8. |
| CLAUDE.md | Updates canonical dev commands and clarifies local vs CI parallelism guidance. |
| .github/CONTRIBUTING.md | Updates GitHub contribution guide pytest commands to -n 8. |
| .claude/skills/pre-pr-review/SKILL.md | Updates the pre-PR verification steps to run pytest with -n 8. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **Coverage**: 80% minimum (enforced in CI) | ||
| - **Parallelism**: always include `-n auto` (pytest-xdist) | ||
| - **Parallelism**: always include `-n 8` (pytest-xdist) |
There was a problem hiding this comment.
The “Parallelism” rule is phrased as an absolute (“always include -n 8”), but elsewhere (e.g., CLAUDE.md) the guidance differentiates local (-n 8) vs CI (-n auto). To avoid contributors misapplying this to CI or other constrained environments, reword this bullet to explicitly scope it to local runs and mention CI’s -n auto behavior (or point to CLAUDE.md for the rule).
| - **Parallelism**: always include `-n 8` (pytest-xdist) | |
| - **Parallelism**: for local runs, include `-n 8` (pytest-xdist); CI may use `-n auto` per project guidance |
There was a problem hiding this comment.
Code Review
This pull request updates various documentation and configuration files to replace the pytest '-n auto' flag with '-n 8' to improve stability on high-core machines. The reviewer noted that the 'pyproject.toml' file still contains the '-n auto' flag in its 'addopts' section, which may lead to inconsistent behavior, and suggested updating this configuration to match the new documentation.
| - **Async**: `asyncio_mode = "auto"` -- no manual `@pytest.mark.asyncio` needed | ||
| - **Timeout**: 30 seconds per test (global in `pyproject.toml` -- do not add per-file `pytest.mark.timeout(30)` markers; non-default overrides like `timeout(60)` are allowed) | ||
| - **Parallelism**: `pytest-xdist` via `-n auto` -- **ALWAYS** include `-n auto` when running pytest, never run tests sequentially | ||
| - **Parallelism**: `pytest-xdist` via `-n 8` -- **ALWAYS** include `-n 8` when running pytest locally, never run tests sequentially. CI uses `-n auto` (fewer cores on runners). |
There was a problem hiding this comment.
The PR description states that all stale -n auto references have been removed, but pyproject.toml still contains -n=auto in the addopts section (line 246). This means that running pytest without explicit flags will still default to auto workers, which may continue to cause the crashes on high-core machines that this PR intends to fix.
Consider updating pyproject.toml to use -n 8 as the default, or removing the -n flag from addopts entirely to ensure the documentation and the actual tool behavior are aligned.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/getting_started.md (1)
40-92: 🧹 Nitpick | 🔵 TrivialConsider adding brief context about parallelism choice.
This file now shows
-n 8in both the smoke test (line 40) and coverage commands (line 92), but doesn't explain why this specific value is used. SinceCLAUDE.mdanddocs/guides/contributing.mddocument the rationale (local uses-n 8, CI uses-n auto), consider adding a brief note or cross-reference for users who might wonder about the parallelism setting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/getting_started.md` around lines 40 - 92, Add a one-sentence note explaining the chosen pytest parallelism flag where `-n 8` appears (e.g., in the smoke test command `uv run python -m pytest tests/ -m unit -n 8` and the coverage command `uv run python -m pytest tests/ -n 8 --cov=...`), stating that local dev defaults to `-n 8` while CI uses `-n auto`, and include a short cross-reference to the existing rationale in CLAUDE.md and docs/guides/contributing.md so readers can find full details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/contributing.md`:
- Line 144: Update the "Parallelism" entry in the contributing guide to mention
the CI difference: keep the recommendation to "always include `-n 8`
(pytest-xdist)" but append a clarifying sentence that CI runs use `-n auto`
(fewer cores on runners), similar to the note in CLAUDE.md; edit the
"Parallelism" bullet (the entry that currently reads "**Parallelism**: always
include `-n 8` (pytest-xdist)") to include that CI uses `-n auto` so
contributors understand the discrepancy.
---
Outside diff comments:
In `@docs/getting_started.md`:
- Around line 40-92: Add a one-sentence note explaining the chosen pytest
parallelism flag where `-n 8` appears (e.g., in the smoke test command `uv run
python -m pytest tests/ -m unit -n 8` and the coverage command `uv run python -m
pytest tests/ -n 8 --cov=...`), stating that local dev defaults to `-n 8` while
CI uses `-n auto`, and include a short cross-reference to the existing rationale
in CLAUDE.md and docs/guides/contributing.md so readers can find full details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78dd2571-52c4-4c85-a27d-ab8d99179998
📒 Files selected for processing (5)
.claude/skills/pre-pr-review/SKILL.md.github/CONTRIBUTING.mdCLAUDE.mddocs/getting_started.mddocs/guides/contributing.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Upload results
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation is built with Zensical from Markdown sources in
docs/. Design spec is indocs/design/(12 pages). Architecture docs indocs/architecture/. REST API reference indocs/rest-api.md(with generateddocs/_generated/api-reference.html). Auto-generated library reference via mkdocstrings + Griffe indocs/api/.
Files:
docs/getting_started.mddocs/guides/contributing.mdCLAUDE.md
🧠 Learnings (33)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Parallelism: pytest-xdist via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: ALWAYS run pytest with `-n auto` for parallel execution with pytest-xdist; never run tests sequentially
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to tests/**/*.py : Always run pytest with `-n auto` for parallel execution via `pytest-xdist`. Never run tests sequentially.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run unit tests with `uv run python -m pytest tests/ -m unit -n auto`; integration tests with `-m integration -n auto`; e2e tests with `-m e2e -n auto`
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run full pytest suite with coverage: `uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80`
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Type-check Python code with `uv run mypy src/ tests/` (strict mode)
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Lint Python code with `uv run ruff check src/ tests/`; auto-fix with `--fix`; format with `uv run ruff format src/ tests/`
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Parallelism: pytest-xdist via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to tests/**/*.py : Test markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`. Coverage: 80% minimum. Async: `asyncio_mode = 'auto'` — no manual `pytest.mark.asyncio` needed. Timeout: 30 seconds per test. Parallelism: `pytest-xdist` via `-n auto` — ALWAYS include `-n auto` when running pytest, never run tests sequentially.
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: ALWAYS run pytest with `-n auto` for parallel execution with pytest-xdist; never run tests sequentially
Applied to files:
docs/getting_started.mddocs/guides/contributing.mdCLAUDE.md
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Test markers: pytest.mark.unit, pytest.mark.integration, pytest.mark.e2e, pytest.mark.slow. Coverage: 80% minimum (enforced in CI).
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to tests/**/*.py : Maintain 80% code coverage minimum (enforced in CI). Never skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic. For tasks that must block indefinitely, use `asyncio.Event().wait()` instead of large `asyncio.sleep()` values.
Applied to files:
docs/getting_started.md.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to pyproject.toml : Dependencies: all versions use == in pyproject.toml. Groups: test (pytest + plugins, hypothesis), dev (includes test + ruff, mypy, pre-commit, commitizen, pip-audit). Required: mem0ai (Mem0 memory backend — the default and currently only backend). Install: uv sync installs everything (dev group is default).
Applied to files:
docs/getting_started.md
📚 Learning: 2026-03-21T12:54:22.557Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T12:54:22.557Z
Learning: Version bumping (pre-1.0): `fix:` = patch, `feat:` = patch, `feat!:` = minor, `BREAKING CHANGE` trailer = minor. Update version in `pyproject.toml` (`[tool.commitizen].version`) and `src/synthorg/__init__.py` (`__version__`)
Applied to files:
.github/CONTRIBUTING.md
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to tests/**/*.py : Always run pytest with `-n auto` for parallel execution via `pytest-xdist`. Never run tests sequentially.
Applied to files:
.github/CONTRIBUTING.mddocs/guides/contributing.mdCLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use pytest markers: `pytest.mark.unit`, `pytest.mark.integration`, `pytest.mark.e2e`, `pytest.mark.slow`
Applied to files:
docs/guides/contributing.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Export OpenAPI schema with `uv run python scripts/export_openapi.py` (required before docs build)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run all pre-commit hooks with `uv run pre-commit run --all-files`
Applied to files:
CLAUDE.md.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Build docs with `uv run zensical build` (output: `_site/docs/`); preview locally with `uv run zensical serve`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; mock `time.monotonic()` and `asyncio.sleep()` for determinism; use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T16:35:04.613Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.613Z
Learning: Pre-push hooks run: mypy type-check (affected modules only) + pytest unit tests (affected modules only) + golangci-lint + go vet + go test (CLI) + eslint-web (web dashboard). Foundational module changes (core, config, observability) or conftest changes trigger full runs. Pre-commit.ci: autoupdate is disabled; Dependabot owns hook version bumps.
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Pre-push hooks enforce: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional) + eslint-web (web dashboard, conditional) — fast gate before push, skipped in pre-commit.ci
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Pre-push hooks: mypy type-check + pytest unit tests + golangci-lint + go vet + go test (CLI, conditional on cli/**/*.go) (fast gate before push, skipped in pre-commit.ci — dedicated CI jobs already run these).
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to .github/workflows/**/*.yml : Use `dorny/paths-filter` for path-based job filtering—jobs only run when their domain is affected. CLI has its own workflow (`cli.yml`). Parallel jobs: lint (ruff) + type-check (mypy) + test (pytest + coverage) + python-audit (pip-audit) + dockerfile-lint (hadolint). Gated by ci-pass check.
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Pre-commit hooks enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint, golangci-lint + go vet (CLI, conditional), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard, zero warnings, conditional)
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-04-02T16:35:04.613Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.613Z
Learning: Never create a PR directly with `gh pr create` (blocked by hookify). Always use `/pre-pr-review` to create PRs—it runs automated checks and review agents before creating the PR. For trivial/docs-only changes, use `/pre-pr-review quick`. After the PR exists, use `/aurelio-review-pr` for external reviewer feedback. Fix all valid issues found by review agents—never skip or defer.
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Pre-commit hooks: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint (Dockerfile linting).
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-04-02T16:35:04.612Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T16:35:04.612Z
Learning: Applies to .pre-commit-config.yaml : Enforce: trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, no-commit-to-branch (main), ruff check+format, gitleaks, hadolint, golangci-lint + go vet (CLI, conditional on `cli/**/*.go`), no-em-dashes, no-redundant-timeout, eslint-web (web dashboard, zero warnings, conditional on `web/src/**/*.{ts,tsx}`).
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
Applied to files:
.claude/skills/pre-pr-review/SKILL.md
🔇 Additional comments (8)
docs/getting_started.md (1)
40-40: Learnings contradict this change and require updating.The retrieved learnings explicitly mandate
-n autofor all pytest runs:
- "ALWAYS run pytest with
-n autofor parallel execution with pytest-xdist; never run tests sequentially"- "Parallelism: pytest-xdist via
-n auto— ALWAYS include-n autowhen running pytest, never run tests sequentially."This PR intentionally changes local development to
-n 8to avoid resource contention on high-core machines, while CI continues using-n auto. The learnings must be updated to reflect this new local vs CI split.Based on learnings: "ALWAYS run pytest with
-n autofor parallel execution with pytest-xdist; never run tests sequentially" and similar statements that are now outdated..github/CONTRIBUTING.md (1)
88-91: Learnings contradict this change and require updating.The retrieved learnings explicitly mandate
-n auto:
- "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto; integration tests with-m integration -n auto; e2e tests with-m e2e -n auto"- "Run full pytest suite with coverage:
uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80"These exact command examples are now changed to use
-n 8for local development. The learnings must be updated to distinguish between local (8 workers) and CI (auto-detection) parallelism.Based on learnings: "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto" and related commands that are now outdated.docs/guides/contributing.md (1)
129-138: Learnings contradict this change and require updating.The retrieved learnings mandate
-n auto:
- "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto"- "Run full pytest suite with coverage:
uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80"These commands are now changed to
-n 8for local development. The learnings must be updated to reflect the new local vs CI parallelism split.Based on learnings: "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto; integration tests with-m integration -n auto; e2e tests with-m e2e -n auto".claude/skills/pre-pr-review/SKILL.md (2)
174-174: Learnings contradict this change and require updating.The retrieved learnings mandate
-n auto:
- "ALWAYS run pytest with
-n autofor parallel execution with pytest-xdist; never run tests sequentially"- "Parallelism: pytest-xdist via
-n auto— ALWAYS include-n autowhen running pytest, never run tests sequentially."The pre-PR pipeline runs locally, so
-n 8is the correct choice per this PR's objectives. The learnings must be updated to distinguish local development (8 workers) from CI (auto-detection).Based on learnings: "ALWAYS run pytest with
-n autofor parallel execution with pytest-xdist; never run tests sequentially"
174-174: LGTM - consistent parallelism across all pre-PR pipeline phases.All three pytest invocations in the pre-PR pipeline (Phase 2 automated checks at line 174, Phase 8 post-fix verification at line 740, and Phase 9 polish pass re-verification at line 761) now consistently use
-n 8for local execution, matching the documented local development standard.Also applies to: 740-740, 761-761
CLAUDE.md (3)
36-40: Learnings contradict this change and require updating.The retrieved learnings mandate
-n auto:
- "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto"- "Run full pytest suite with coverage:
uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80"- "Run property tests with
HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties"All these commands are now changed to
-n 8for local development. The learnings must be updated to reflect the new parallelism policy.Based on learnings: "Run unit tests with
uv run python -m pytest tests/ -m unit -n auto; integration tests with-m integration -n auto; e2e tests with-m e2e -n auto" and related command examples.
170-170: LGTM - comprehensive parallelism documentation.Line 170 now provides the complete context: local development uses
-n 8while CI uses-n auto(fewer cores on runners). This explanation helps developers understand the rationale and prevents confusion about the discrepancy between local commands and CI workflow configuration.
36-40: LGTM - consistent parallelism across all pytest commands.All pytest command examples in the Quick Commands section (unit, integration, e2e, full suite with coverage, and Hypothesis property tests) now consistently use
-n 8, and the Testing section at line 170 documents the rationale with the CI comparison. This provides a complete, self-contained reference.Also applies to: 170-173
|
|
||
| - **Coverage**: 80% minimum (enforced in CI) | ||
| - **Parallelism**: always include `-n auto` (pytest-xdist) | ||
| - **Parallelism**: always include `-n 8` (pytest-xdist) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider documenting the CI difference.
Line 144 now states "always include -n 8 (pytest-xdist)" as the parallelism rule, but doesn't mention that CI uses -n auto. CLAUDE.md includes this clarification ("CI uses -n auto (fewer cores on runners)"), which helps users understand the split. Consider adding a similar note here for completeness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/contributing.md` at line 144, Update the "Parallelism" entry in
the contributing guide to mention the CI difference: keep the recommendation to
"always include `-n 8` (pytest-xdist)" but append a clarifying sentence that CI
runs use `-n auto` (fewer cores on runners), similar to the note in CLAUDE.md;
edit the "Parallelism" bullet (the entry that currently reads "**Parallelism**:
always include `-n 8` (pytest-xdist)") to include that CI uses `-n auto` so
contributors understand the discrepancy.
The actual pytest default was still -n=auto in addopts, making the documentation changes ineffective for bare pytest invocations. Found by: gemini-code-assist review on #1013
The pre-push hook script had 'auto' hardcoded as the xdist worker count, bypassing the pyproject.toml addopts change from #1013.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1013 +/- ##
=======================================
Coverage 91.66% 91.66%
=======================================
Files 656 656
Lines 35719 35719
Branches 3512 3512
=======================================
Hits 32743 32743
Misses 2357 2357
Partials 619 619 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
## Summary The pre-push hook script `scripts/run_affected_tests.py` had `"auto"` hardcoded as the xdist worker count (line 160), bypassing the `pyproject.toml` addopts change from #1013. This was missed in #1013 because the value was split across two Python list elements (`"-n"`, `"auto"`) rather than a single string `"-n auto"`, so the grep search didn't catch it. ### Changes - `scripts/run_affected_tests.py`: Change worker count from `"auto"` to `"8"` ### Why Follow-up to #1013. Without this fix, `git push` still triggers 32 pytest workers via the pre-push hook, causing the same crashes and slowness that #1013 aimed to fix.
Summary
Reduce local pytest worker count from
-n auto(32 workers on dev machine) to-n 8to fix crashes and improve test speed.Changes
-n 8, parallelism rule clarified (local =-n 8, CI =-n auto)-n 8-n 8-n 8-n 8Why
On machines with 32 logical CPUs,
-n autospawns 32 pytest-xdist workers which causes:8 workers (1/4 of CPU count) is the sweet spot for Python test suites. CI keeps
-n autosince GitHub Actions runners have 2-4 cores where auto is appropriate.Not changed (intentionally)
.github/workflows/ci.ymlkeeps-n auto(CI runners have fewer cores).local.md, gitignored) updated separatelyTest plan
Documentation-only change -- no code affected. Verified no stale
-n autoreferences remain in project files (only CI workflow and explanatory mentions).