diff --git a/.agents/recipes/_fix-policy.md b/.agents/recipes/_fix-policy.md index 8bcbea9bc..f9f9ffd7a 100644 --- a/.agents/recipes/_fix-policy.md +++ b/.agents/recipes/_fix-policy.md @@ -11,7 +11,9 @@ When in doubt, fall back to report-only. A finding may be converted to a fix only if all hold: -- **Bounded scope**: ≤3 files, ≤50 LOC net. +- **Bounded scope**: ≤3 files, ≤50 LOC net. For the dependencies suite, + exclude generated `uv.lock` changes from the LOC total; the lockfile still + counts toward the file cap and must pass the path allowlist. - **Reversible**: no public API changes, no `__all__` deletions, no version bumps (Dependabot owns those), no schema changes, no migrations. - **Self-evident**: the audit established both the problem *and* the unique @@ -40,7 +42,7 @@ If the top-ranked candidate fails the bar, try the next. If none of the top | Suite | Paths the recipe MAY modify | |-------|-----------------------------| | docs-and-references | `architecture/**`, `fern/versions/latest/pages/**`, `README.md`, `CONTRIBUTING.md`, `DEVELOPMENT.md`, `STYLEGUIDE.md`, `packages/*/src/**/*.py` (docstring-only edits) | -| dependencies | `packages/*/pyproject.toml` | +| dependencies | `packages/*/pyproject.toml`, `uv.lock` | | structure | `packages/*/src/**/*.py` | | code-quality | `packages/*/src/**/*.py` | | test-health | (no fix phase) | @@ -83,6 +85,8 @@ code-quality and unset elsewhere) controls draft-PR mode. Batch PRs still record one `attempted_fixes` entry per finding. Multiple entries may point to the same `pr_number` and `branch`. +Post-fix gates group those entries by `(pr_number, branch)`, validate the +shared diff once, and update every associated finding together on rejection. ### `fix_backlog` rules (audit phase populates this) @@ -174,9 +178,9 @@ Earlier criteria override later ones: | Severity | Examples | |----------|----------| - | high | missing transitive dep, heavy import bypassing lazy system | + | high | imported dependency not guaranteed by any required package, heavy import bypassing lazy system | | medium | broken doc link visible on docs site, bare-except hiding errors, docstring drift on public API | - | low | broken link in dev-notes, missing `__future__ import annotations`, unused dep | + | low | direct-dependency gap already guaranteed by a required package, broken link in dev-notes, missing `__future__ import annotations`, unused dep | 3. **User-facing impact** — visible to docs-site readers or plugin consumers vs internal-only. @@ -202,9 +206,9 @@ declare only the parts that vary (eligible categories, branch type, 4. For each primary candidate, top 5 max: 1. If the suite declares the category batchable, collect sibling `fix_backlog` entries for the same suite/category that share the same - test target and branch type. Do not discover new findings; use only - existing backlog entries. Batch at most 3 entries to stay within the - localized-fix file cap. + target package, test target, and branch type. Do not discover new + findings; use only existing backlog entries. Batch at most 3 entries + to stay within the localized-fix file cap. 2. Re-verify every finding still applies (re-grep / re-read). If a sibling no longer applies, remove it from `fix_backlog`; if the primary no longer applies, remove it from `fix_backlog` and continue diff --git a/.agents/recipes/dependencies/recipe.md b/.agents/recipes/dependencies/recipe.md index bc72ee805..9e0aebf10 100644 --- a/.agents/recipes/dependencies/recipe.md +++ b/.agents/recipes/dependencies/recipe.md @@ -45,30 +45,35 @@ Note: engine and interface packages use `uv-dynamic-versioning` to inject dependencies. Check both static declarations and the dynamic versioning config. -### 2. Transitive dependency gaps +### 2. Direct dependency declaration gaps -This is the highest-value check. A package may import a library that it -doesn't declare as a dependency, relying on another package to pull it in -transitively. This works until the packages are installed separately. +Read `/tmp/dependency-inventory.json`, generated deterministically before this +recipe by `scripts/audit_package_dependencies.py`. It uses Python ASTs and +installed distribution metadata to compare imports with static and dynamic +package declarations. -For each package, verify that every imported third-party library is declared -in that package's own `[project.dependencies]`: +For every entry in `packages[].missing`: -```bash -# Find all third-party imports in engine source -grep -rhn "^import \|^from " packages/data-designer-engine/src/ --include='*.py' \ - | grep -v "data_designer" | grep -v "^from __future__" \ - | sort -u +- Re-read the listed source files to confirm the import is runtime code rather + than a guarded optional import. +- Use `declared_by` to find a sibling package's existing version specifier. +- Treat `severity: low` entries as metadata hygiene: `guaranteed_by` names a + mandatory workspace dependency that already installs the library. Do not + claim these currently break standalone installation. +- Treat high severity as a candidate classification. Keep it high only after + confirming the import is required runtime code; testing helpers and optional + integrations stay report-only unless their packaging contract requires the + dependency. -# Compare against declared deps in engine's pyproject.toml -``` +Review `unresolved_modules` manually. Report a gap only after mapping the +module to a distribution with repository evidence. Never guess a package name. -Known issue to check: `numpy` and `pandas` are used by the engine but may -only be declared in the config package. Each package should declare what it -directly imports. +Only add a gap to `fix_backlog` when `declared_by` identifies a sibling +specifier that can be copied mechanically. Gaps without a sibling declaration +are report-only and must not consume the fix phase's top-five candidate limit. -Also check lazy imports in `data_designer/lazy_heavy_imports.py` - these -are intentionally deferred but still need to be declared as dependencies. +Also check lazy imports in `data_designer/lazy_heavy_imports.py`; these are +intentionally deferred but still need to be declared as dependencies. ### 3. Cross-package version consistency @@ -87,12 +92,10 @@ they haven't been bypassed by a looser pin elsewhere. ### 4. Unused dependency detection -For each declared dependency, check if it is actually imported anywhere in -the corresponding package: -```bash -# Example: check if 'lxml' is imported in data-designer-engine -grep -r "import lxml\|from lxml" packages/data-designer-engine/src/ -``` +Use each inventory entry's `declared` and `imported` lists to seed unused +dependency candidates. The inventory is not proof of non-use: workspace +package dependencies, lazy imports, plugins, command entry points, and +runtime-only requirements may not appear as ordinary imports. A dependency is "unused" if: - Not imported directly anywhere in the package source @@ -124,11 +127,11 @@ Write the report to `/tmp/audit-{{suite}}.md`: ## Dependency Audit - {{date}} -### Transitive dependency gaps +### Direct dependency declaration gaps -| Package | Import | Declared in | Should be declared in | -|---------|--------|-------------|----------------------| -| engine | numpy | config only | engine (direct import in ...) | +| Package | Import | Guaranteed by | Severity | Should be declared in | +|---------|--------|---------------|----------|-----------------------| +| engine | numpy | config | low | engine (direct import in ...) | ### Cross-package inconsistencies @@ -150,7 +153,7 @@ Write the report to `/tmp/audit-{{suite}}.md`: ### Summary -- N transitive gaps (M new since last run) +- N direct dependency declaration gaps (M new since last run) - N cross-package inconsistencies - N unused dependencies (M new) - N pinning concerns @@ -164,21 +167,26 @@ Follow the standard fix procedure in `_fix-policy.md`. Suite-specific bits: ### Eligible categories -| Category | Branch type | test_required | Eligibility note | -|----------|-------------|---------------|------------------| -| transitive-gap | `chore` | yes | Add the imported module to `[project.dependencies]` of the package that imports it, copying the version specifier from a sibling package that already declares it. Insert in alphabetical order; match existing quote/specifier style. **Ineligible** when no sibling package declares the dep — choosing a specifier from scratch is interpretive, not mechanical. Those findings stay report-only and surface for maintainer judgement. | -| unused | `chore` | yes | Remove the declaration. Eligible only when grep across the package's `src/`, lazy-import system, plugin entry points, and tests turns up zero references. | +| Category | Branch type | test_required | Batchable | Eligibility note | +|----------|-------------|---------------|-----------|------------------| +| transitive-gap | `chore` | yes | yes, by target package (max 3) | Add the imported distribution to the dependency list of the package that imports it, copying the version specifier from a sibling package that already declares it. Insert in alphabetical order; match existing quote/specifier style. **Ineligible** when no sibling package declares the dep — choosing a specifier from scratch is interpretive, not mechanical. Those findings stay report-only and surface for maintainer judgement. | +| unused | `chore` | yes | no | Remove the declaration. Eligible only when checks across the package's `src/`, lazy-import system, plugin entry points, and tests turn up zero references. | + +`fix_backlog.data` should record: for transitive-gap, the target package, +dependency name, importing source files, sibling package whose specifier was +copied, `guaranteed_by`, severity, and test target. The recipe must record this +during the audit; the fix phase rejects entries with no sibling source. For +unused, record which other packages also declare the dependency. -`fix_backlog.data` should record: for transitive-gap, the importing source -files and the sibling package whose specifier was copied (the recipe -must record this *during the audit*; the fix phase rejects entries with -no sibling source). For unused, which other packages also declare the -dep. +Batch transitive gaps only when they target the same package and test target. +Apply at most three dependency declarations in one PR. Include one hidden +finding marker and one `attempted_fixes` entry per dependency. -Before running the per-package test target (see `_fix-policy.md` for the -mapping), run `make install-dev` to confirm the lockfile resolves cleanly. -`make install-dev` is the only sanctioned install command (no direct -`pip install` or `uv pip install`). +After editing the package manifest, run `make install-dev`, commit the +regenerated `uv.lock`, and run `uv lock --check`. The PR must include +`uv.lock`; a manifest-only dependency PR is incomplete. Then run the +per-package test target (see `_fix-policy.md`). `make install-dev` is the only +sanctioned install command (no direct `pip install` or `uv pip install`). **Not eligible** — stays report-only: @@ -189,8 +197,8 @@ mapping), run `make install-dev` to confirm the lockfile resolves cleanly. ## Constraints - Outside the fix phase, this recipe is read-only — do not modify files. -- Within the fix phase, only modify `packages/*/pyproject.toml`. The - repo-root `pyproject.toml` is forbidden. +- Within the fix phase, only modify `packages/*/pyproject.toml` and `uv.lock`. + The repo-root `pyproject.toml` is forbidden. - `make install-dev` is the only sanctioned install command. Do not invoke `pip install` or `uv pip install` directly. - Do not run `pip audit` (may not be available on the runner). Focus on diff --git a/.github/workflows/agentic-ci-daily.yml b/.github/workflows/agentic-ci-daily.yml index 1eddd8064..ac1ce68c7 100644 --- a/.github/workflows/agentic-ci-daily.yml +++ b/.github/workflows/agentic-ci-daily.yml @@ -131,6 +131,14 @@ jobs: python -m venv /tmp/graphify-venv /tmp/graphify-venv/bin/python -m pip install graphifyy==0.4.23 --quiet 2>&1 | tail -3 + - name: Build dependency inventory + if: matrix.suite == 'dependencies' + run: | + .venv/bin/python scripts/audit_package_dependencies.py \ + --output /tmp/dependency-inventory.json + jq '.packages[] | {package, missing, unresolved_modules}' \ + /tmp/dependency-inventory.json + - name: Configure git identity run: | git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -288,31 +296,25 @@ jobs: set -o pipefail # Identify every attempted_fixes entry that grew during *this* run - # (vs the pre-fix snapshot), not just the last globally-open entry. - OPEN_ENTRIES=$(jq -c --slurpfile prior /tmp/prior-attempted-fixes.json ' - (($prior[0] // []) | map({key: .id, value: .n}) | from_entries) as $p - | [ - .attempted_fixes // [] - | .[] - | select( - ((.attempts | last | .outcome) == "open") - and ((.attempts | length) > ($p[.id] // 0)) - ) - ] - ' .agentic-ci-state/runner-state.json) - OPEN_COUNT=$(echo "$OPEN_ENTRIES" | jq 'length') + # and group entries sharing one pushed PR/branch. + OPEN_GROUPS=$(python3 scripts/manage_agentic_ci_attempts.py groups \ + --state .agentic-ci-state/runner-state.json \ + --prior /tmp/prior-attempted-fixes.json) + OPEN_COUNT=$(echo "$OPEN_GROUPS" | jq '[.[].finding_ids[]] | length') + GROUP_COUNT=$(echo "$OPEN_GROUPS" | jq 'length') if [ "$OPEN_COUNT" -eq 0 ]; then echo "No new open attempted_fix recorded by this run; nothing to validate." exit 0 fi - echo "Validating ${OPEN_COUNT} new open attempted_fix entries." + echo "Validating ${OPEN_COUNT} new open attempted_fix entries across ${GROUP_COUNT} PR group(s)." REJECTED=0 - while IFS= read -r OPEN; do - BRANCH=$(echo "$OPEN" | jq -r '.attempts | last | .branch // empty') - PR_NUMBER=$(echo "$OPEN" | jq -r '.attempts | last | .pr_number // empty') - FINDING_ID=$(echo "$OPEN" | jq -r '.id') + while IFS= read -r GROUP; do + BRANCH=$(echo "$GROUP" | jq -r '.branch // empty') + PR_NUMBER=$(echo "$GROUP" | jq -r '.pr_number // empty') + FINDING_IDS=$(echo "$GROUP" | jq -c '.finding_ids') + FINDING_LABEL=$(echo "$GROUP" | jq -r '.finding_ids | join(", ")') DIFF_REF="" REASONS="" @@ -342,7 +344,7 @@ jobs: ALLOW='^(architecture/|docs/|README\.md$|CONTRIBUTING\.md$|DEVELOPMENT\.md$|STYLEGUIDE\.md$|packages/[^/]+/src/.*\.py$)' ;; dependencies) - ALLOW='^packages/[^/]+/pyproject\.toml$' + ALLOW='^(packages/[^/]+/pyproject\.toml|uv\.lock)$' ;; structure|code-quality) ALLOW='^packages/[^/]+/src/.*\.py$' @@ -361,7 +363,11 @@ jobs: else BAD="" fi - LOC_DELTA=$(git diff --shortstat "origin/main...$DIFF_REF" \ + LOC_PATHSPEC=() + if [ "$SUITE" = "dependencies" ]; then + LOC_PATHSPEC=(-- ':(exclude)uv.lock') + fi + LOC_DELTA=$(git diff --shortstat "origin/main...$DIFF_REF" "${LOC_PATHSPEC[@]}" \ | awk '{ a=0; d=0; for (i=1;i<=NF;i++) { if ($i ~ /insertion/) a=$(i-1); if ($i ~ /deletion/) d=$(i-1) } print a+d }') : "${LOC_DELTA:=0}" else @@ -495,16 +501,16 @@ jobs: fi if [ -z "$REASONS" ]; then - echo "Scope gate passed for ${FINDING_ID}: ${FILE_COUNT} file(s), ${LOC_DELTA} LOC, all within allowlist." + echo "Scope gate passed for ${FINDING_LABEL}: ${FILE_COUNT} file(s), ${LOC_DELTA} LOC, all within allowlist." continue fi REJECTED=1 - echo "::error::Scope gate violation for ${FINDING_ID}" + echo "::error::Scope gate violation for ${FINDING_LABEL}" printf '%b' "$REASONS" if [ -n "$PR_NUMBER" ] && [ "$PR_NUMBER" != "null" ]; then - MSG=$(printf 'Closed by workflow scope gate. The pushed diff violated the localized-fix bar (see `.agents/recipes/_fix-policy.md`):\n\n%b\nThe `attempted_fixes` entry has been flipped to `abandoned`.' "$REASONS") + MSG=$(printf 'Closed by workflow scope gate. The pushed diff violated the localized-fix bar (see `.agents/recipes/_fix-policy.md`):\n\n%b\nAll associated `attempted_fixes` entries have been flipped to `abandoned`.' "$REASONS") gh pr close "$PR_NUMBER" --comment "$MSG" --delete-branch || \ echo "::warning::gh pr close failed; branch may need manual cleanup" elif [ -n "$BRANCH" ]; then @@ -514,26 +520,12 @@ jobs: echo "::warning::No PR number or branch available for cleanup" fi - FINDING_ID="$FINDING_ID" python3 - <<'PY' - import json, os - finding_id = os.environ['FINDING_ID'] - path = '.agentic-ci-state/runner-state.json' - with open(path) as f: - state = json.load(f) - for entry in state.get('attempted_fixes', []): - if entry.get('id') != finding_id: - continue - attempts = entry.get('attempts') or [] - if attempts and attempts[-1].get('outcome') == 'open': - attempts[-1]['outcome'] = 'abandoned' - attempts[-1]['gate_violation'] = True - tmp = path + '.tmp' - with open(tmp, 'w') as f: - json.dump(state, f, indent=2) - os.replace(tmp, path) - PY + python3 scripts/manage_agentic_ci_attempts.py abandon \ + --state .agentic-ci-state/runner-state.json \ + --finding-ids "$FINDING_IDS" \ + --marker gate_violation - done < <(echo "$OPEN_ENTRIES" | jq -c '.[]') + done < <(echo "$OPEN_GROUPS" | jq -c '.[]') if [ "$REJECTED" -eq 1 ]; then echo "rejected=true" >> "$GITHUB_OUTPUT" @@ -542,9 +534,7 @@ jobs: - name: Verify dependencies lockfile # Dependencies suite only: re-run make install-dev against the - # agent's pyproject.toml changes. This catches the failure mode - # where the per-package test target passed against the *old* - # lockfile but the proposed dep does not actually resolve. + # pushed branch and require the committed uv.lock to remain clean. id: lockfile_gate if: always() && matrix.suite == 'dependencies' && steps.snapshot.outcome == 'success' && steps.scope_gate.outcome == 'success' env: @@ -552,57 +542,27 @@ jobs: run: | set -o pipefail - # Same snapshot-based selector as the scope gate: target every - # entry whose attempts grew during this run. - OPEN_ENTRIES=$(jq -c --slurpfile prior /tmp/prior-attempted-fixes.json ' - (($prior[0] // []) | map({key: .id, value: .n}) | from_entries) as $p - | [ - .attempted_fixes // [] - | .[] - | select( - ((.attempts | last | .outcome) == "open") - and ((.attempts | length) > ($p[.id] // 0)) - ) - ] - ' .agentic-ci-state/runner-state.json) - OPEN_COUNT=$(echo "$OPEN_ENTRIES" | jq 'length') + # Same snapshot-based selector as the scope gate: target each + # pushed PR/branch group once. + OPEN_GROUPS=$(python3 scripts/manage_agentic_ci_attempts.py groups \ + --state .agentic-ci-state/runner-state.json \ + --prior /tmp/prior-attempted-fixes.json) + OPEN_COUNT=$(echo "$OPEN_GROUPS" | jq '[.[].finding_ids[]] | length') + GROUP_COUNT=$(echo "$OPEN_GROUPS" | jq 'length') if [ "$OPEN_COUNT" -eq 0 ]; then echo "No new open attempted_fix; skipping lockfile verification." exit 0 fi - echo "Verifying dependencies lockfile for ${OPEN_COUNT} new open attempted_fix entries." + echo "Verifying dependencies lockfile for ${OPEN_COUNT} new open attempted_fix entries across ${GROUP_COUNT} PR group(s)." REJECTED=0 BASE_REF=$(git rev-parse HEAD) - abandon_open_attempt() { - local finding_id="$1" - local marker="$2" - FINDING_ID="$finding_id" MARKER="$marker" python3 - <<'PY' - import json, os - finding_id = os.environ['FINDING_ID'] - marker = os.environ['MARKER'] - path = '.agentic-ci-state/runner-state.json' - with open(path) as f: - state = json.load(f) - for entry in state.get('attempted_fixes', []): - if entry.get('id') != finding_id: - continue - attempts = entry.get('attempts') or [] - if attempts and attempts[-1].get('outcome') == 'open': - attempts[-1]['outcome'] = 'abandoned' - attempts[-1][marker] = True - tmp = path + '.tmp' - with open(tmp, 'w') as f: - json.dump(state, f, indent=2) - os.replace(tmp, path) - PY - } - - while IFS= read -r OPEN; do - PR_NUMBER=$(echo "$OPEN" | jq -r '.attempts | last | .pr_number // empty') - BRANCH=$(echo "$OPEN" | jq -r '.attempts | last | .branch // empty') - FINDING_ID=$(echo "$OPEN" | jq -r '.id') + while IFS= read -r GROUP; do + PR_NUMBER=$(echo "$GROUP" | jq -r '.pr_number // empty') + BRANCH=$(echo "$GROUP" | jq -r '.branch // empty') + FINDING_IDS=$(echo "$GROUP" | jq -c '.finding_ids') + FINDING_LABEL=$(echo "$GROUP" | jq -r '.finding_ids | join(", ")') CHECKOUT_REASON="" # Verify against the actually-pushed branch, not local HEAD. @@ -633,17 +593,54 @@ jobs: else echo "::warning::No PR number or branch available for cleanup" fi - abandon_open_attempt "$FINDING_ID" "lockfile_checkout_failed" + python3 scripts/manage_agentic_ci_attempts.py abandon \ + --state .agentic-ci-state/runner-state.json \ + --finding-ids "$FINDING_IDS" \ + --marker lockfile_checkout_failed + continue + fi + + if ! git diff --name-only origin/main...HEAD -- uv.lock | grep -qx 'uv.lock'; then + REJECTED=1 + echo "::error::Dependency PR for ${FINDING_LABEL} does not include uv.lock" + if [ -n "$PR_NUMBER" ] && [ "$PR_NUMBER" != "null" ]; then + MSG="Closed by workflow lockfile verification. Dependency PRs must include the regenerated \`uv.lock\`." + gh pr close "$PR_NUMBER" --comment "$MSG" --delete-branch || \ + echo "::warning::gh pr close failed" + elif [ -n "$BRANCH" ]; then + git push origin --delete "$BRANCH" || true + fi + python3 scripts/manage_agentic_ci_attempts.py abandon \ + --state .agentic-ci-state/runner-state.json \ + --finding-ids "$FINDING_IDS" \ + --marker lockfile_missing continue fi if make install-dev 2>&1 | tee /tmp/install-dev-verify.log; then - echo "Lockfile resolves cleanly for ${FINDING_ID}." + if git diff --quiet -- uv.lock; then + echo "Committed lockfile resolves cleanly for ${FINDING_LABEL}." + continue + fi + + REJECTED=1 + echo "::error::uv.lock changed after make install-dev for ${FINDING_LABEL}" + if [ -n "$PR_NUMBER" ] && [ "$PR_NUMBER" != "null" ]; then + MSG="Closed by workflow lockfile verification. \`make install-dev\` changed \`uv.lock\`; dependency PRs must commit the regenerated lockfile." + gh pr close "$PR_NUMBER" --comment "$MSG" --delete-branch || \ + echo "::warning::gh pr close failed" + elif [ -n "$BRANCH" ]; then + git push origin --delete "$BRANCH" || true + fi + python3 scripts/manage_agentic_ci_attempts.py abandon \ + --state .agentic-ci-state/runner-state.json \ + --finding-ids "$FINDING_IDS" \ + --marker lockfile_not_committed continue fi REJECTED=1 - echo "::error::make install-dev failed against the agent's pyproject changes for ${FINDING_ID}" + echo "::error::make install-dev failed against the agent's pyproject changes for ${FINDING_LABEL}" if [ -n "$PR_NUMBER" ] && [ "$PR_NUMBER" != "null" ]; then MSG="Closed by workflow lockfile verification. \`make install-dev\` failed against the agent's \`pyproject.toml\` changes — the dependency edit does not resolve cleanly. See \`/tmp/install-dev-verify.log\` in the workflow artifact." @@ -655,9 +652,12 @@ jobs: echo "::warning::No PR number or branch available for cleanup" fi - abandon_open_attempt "$FINDING_ID" "lockfile_verification_failed" + python3 scripts/manage_agentic_ci_attempts.py abandon \ + --state .agentic-ci-state/runner-state.json \ + --finding-ids "$FINDING_IDS" \ + --marker lockfile_verification_failed - done < <(echo "$OPEN_ENTRIES" | jq -c '.[]') + done < <(echo "$OPEN_GROUPS" | jq -c '.[]') if [ "$REJECTED" -eq 1 ]; then echo "rejected=true" >> "$GITHUB_OUTPUT" @@ -701,6 +701,7 @@ jobs: /tmp/claude-audit-log.txt /tmp/claude-fix-log.txt /tmp/audit-${{ matrix.suite }}.md + /tmp/dependency-inventory.json /tmp/pr-body-${{ matrix.suite }}.md /tmp/install-dev-verify.log .agentic-ci-state/runner-state.json diff --git a/packages/data-designer/tests/test_agentic_ci_attempts.py b/packages/data-designer/tests/test_agentic_ci_attempts.py new file mode 100644 index 000000000..65e6b852e --- /dev/null +++ b/packages/data-designer/tests/test_agentic_ci_attempts.py @@ -0,0 +1,82 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import importlib.util +import json +from pathlib import Path +from types import ModuleType + +import pytest + +SCRIPT_PATH = Path(__file__).parents[3] / "scripts" / "manage_agentic_ci_attempts.py" +SPEC = importlib.util.spec_from_file_location("manage_agentic_ci_attempts", SCRIPT_PATH) +assert SPEC and SPEC.loader +ATTEMPTS = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(ATTEMPTS) + + +@pytest.fixture +def runner_state(tmp_path: Path) -> tuple[Path, Path]: + state_path = tmp_path / "runner-state.json" + prior_path = tmp_path / "prior.json" + state_path.write_text( + json.dumps( + { + "attempted_fixes": [ + { + "id": finding_id, + "attempts": [ + { + "outcome": "open", + "pr_number": 123, + "branch": "agentic-ci/chore/dependencies-batch", + } + ], + } + for finding_id in ("first", "second") + ] + } + ) + ) + prior_path.write_text(json.dumps([{"id": "first", "n": 0}, {"id": "second", "n": 0}])) + return state_path, prior_path + + +def test_groups_batched_attempts(runner_state: tuple[Path, Path]) -> None: + state_path, prior_path = runner_state + module = ATTEMPTS + assert isinstance(module, ModuleType) + + assert module.new_open_attempt_groups(state_path, prior_path) == [ + { + "pr_number": 123, + "branch": "agentic-ci/chore/dependencies-batch", + "finding_ids": ["first", "second"], + } + ] + + +def test_abandons_every_attempt_in_rejected_batch(runner_state: tuple[Path, Path]) -> None: + state_path, _ = runner_state + module = ATTEMPTS + assert isinstance(module, ModuleType) + + module.abandon_open_attempts(state_path, ["first", "second"], "lockfile_missing") + + attempts = [entry["attempts"][-1] for entry in json.loads(state_path.read_text())["attempted_fixes"]] + assert attempts == [ + { + "outcome": "abandoned", + "pr_number": 123, + "branch": "agentic-ci/chore/dependencies-batch", + "lockfile_missing": True, + }, + { + "outcome": "abandoned", + "pr_number": 123, + "branch": "agentic-ci/chore/dependencies-batch", + "lockfile_missing": True, + }, + ] diff --git a/packages/data-designer/tests/test_dependency_audit.py b/packages/data-designer/tests/test_dependency_audit.py new file mode 100644 index 000000000..509433f94 --- /dev/null +++ b/packages/data-designer/tests/test_dependency_audit.py @@ -0,0 +1,146 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import importlib.util +from pathlib import Path +from types import ModuleType + +import pytest + +SCRIPT_PATH = Path(__file__).parents[3] / "scripts" / "audit_package_dependencies.py" +SPEC = importlib.util.spec_from_file_location("audit_package_dependencies", SCRIPT_PATH) +assert SPEC and SPEC.loader +DEPENDENCY_AUDIT = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(DEPENDENCY_AUDIT) + + +@pytest.fixture +def workspace(tmp_path: Path) -> Path: + config = tmp_path / "packages" / "data-designer-config" + engine = tmp_path / "packages" / "data-designer-engine" + (config / "src").mkdir(parents=True) + (engine / "src").mkdir(parents=True) + (config / "pyproject.toml").write_text( + '[project]\nname = "data-designer-config"\ndependencies = ["jinja2>=3.1.6,<4"]\n' + ) + (engine / "pyproject.toml").write_text( + """ +[project] +name = "data-designer-engine" +dynamic = ["dependencies"] + +[tool.hatch.metadata.hooks.uv-dynamic-versioning] +dependencies = ["data-designer-config=={{ version }}"] +""".lstrip() + ) + (engine / "src" / "engine.py").write_text( + "from __future__ import annotations\n\nimport os\nfrom jinja2 import Template\n" + ) + return tmp_path + + +def audit( + workspace: Path, + distributions: dict[str, list[str]], + requirements: dict[str, list[str]] | None = None, +) -> dict: + module = DEPENDENCY_AUDIT + assert isinstance(module, ModuleType) + return module.audit_repository(workspace, distributions, requirements) + + +def test_marks_transitively_guaranteed_gap_low(workspace: Path) -> None: + result = audit(workspace, {"jinja2": ["Jinja2"]}) + + engine = next(package for package in result["packages"] if package["package"] == "data-designer-engine") + assert engine["missing"] == [ + { + "dependency": "jinja2", + "modules": ["jinja2"], + "files": ["packages/data-designer-engine/src/engine.py"], + "declared_by": ["data-designer-config"], + "guaranteed_by": ["data-designer-config"], + "severity": "low", + } + ] + + +def test_marks_uncovered_gap_high(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text('[project]\nname = "example"\ndependencies = []\n') + (package / "src" / "example.py").write_text("import httpx\n") + + result = audit(tmp_path, {"httpx": ["httpx"]}) + + assert result["packages"][0]["missing"][0]["severity"] == "high" + assert result["packages"][0]["missing"][0]["guaranteed_by"] == [] + + +def test_marks_external_transitive_gap_low(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text('[project]\nname = "example"\ndependencies = ["typer"]\n') + (package / "src" / "example.py").write_text("import click\n") + + result = audit(tmp_path, {"click": ["click"]}, {"typer": ["click"]}) + + assert result["packages"][0]["missing"][0]["severity"] == "low" + assert result["packages"][0]["missing"][0]["guaranteed_by"] == ["typer"] + + +def test_marks_dependency_from_selected_extra_low(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text('[project]\nname = "example"\ndependencies = ["pydantic[email]"]\n') + (package / "src" / "example.py").write_text("import email_validator\n") + + result = audit( + tmp_path, + {"email_validator": ["email-validator"]}, + {"pydantic": ['email-validator; extra == "email"']}, + ) + + assert result["packages"][0]["missing"][0]["severity"] == "low" + assert result["packages"][0]["missing"][0]["guaranteed_by"] == ["pydantic"] + + +def test_ignores_inactive_project_marker(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text( + '[project]\nname = "example"\ndependencies = ["typing-extensions; python_version < \'1\'"]\n' + ) + (package / "src" / "example.py").write_text("import typing_extensions\n") + + result = audit(tmp_path, {"typing_extensions": ["typing-extensions"]}) + + assert result["packages"][0]["missing"][0]["severity"] == "high" + assert result["packages"][0]["missing"][0]["guaranteed_by"] == [] + + +def test_reports_ambiguous_module_as_unresolved(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text('[project]\nname = "example"\ndependencies = []\n') + (package / "src" / "example.py").write_text("import shared_namespace\n") + + result = audit(tmp_path, {"shared_namespace": ["first-package", "second-package"]}) + + assert result["packages"][0]["missing"] == [] + assert result["packages"][0]["unresolved_modules"] == [ + {"module": "shared_namespace", "files": ["packages/example/src/example.py"]} + ] + + +def test_applies_module_distribution_override(tmp_path: Path) -> None: + package = tmp_path / "packages" / "example" + (package / "src").mkdir(parents=True) + (package / "pyproject.toml").write_text('[project]\nname = "example"\ndependencies = []\n') + (package / "src" / "example.py").write_text("import yaml\n") + + result = audit(tmp_path, {}) + + assert result["packages"][0]["missing"][0]["dependency"] == "pyyaml" diff --git a/scripts/audit_package_dependencies.py b/scripts/audit_package_dependencies.py new file mode 100644 index 000000000..f044646d6 --- /dev/null +++ b/scripts/audit_package_dependencies.py @@ -0,0 +1,257 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import argparse +import ast +import json +import re +import sys +from collections import defaultdict +from importlib.metadata import PackageNotFoundError, distribution, packages_distributions +from pathlib import Path +from typing import Any + +from packaging.requirements import InvalidRequirement, Requirement + +try: + import tomllib +except ModuleNotFoundError: + import tomli as tomllib + +DEPENDENCY_NAME = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*") +MODULE_DISTRIBUTION_OVERRIDES = { + "IPython": "ipython", + "PIL": "pillow", + "dateutil": "python-dateutil", + "jwt": "pyjwt", + "yaml": "pyyaml", +} + + +def normalize_name(name: str) -> str: + return re.sub(r"[-_.]+", "-", name).lower() + + +def dependency_name(specifier: str) -> str | None: + match = DEPENDENCY_NAME.match(specifier) + return normalize_name(match.group()) if match else None + + +def requirement_dependencies( + specifiers: list[str], + selected_extras: set[str] | None = None, + *, + allow_version_template: bool = False, +) -> dict[str, set[str]]: + dependencies: dict[str, set[str]] = defaultdict(set) + marker_extras = {"", *(selected_extras or set())} + + for specifier in specifiers: + try: + requirement = Requirement(specifier) + except InvalidRequirement: + if not allow_version_template or "{{ version }}" not in specifier: + raise + if name := dependency_name(specifier): + dependencies[name] + continue + + # Marker evaluation is intentionally relative to the current audit environment. + if requirement.marker and not any(requirement.marker.evaluate({"extra": extra}) for extra in marker_extras): + continue + dependencies[normalize_name(requirement.name)].update(normalize_name(extra) for extra in requirement.extras) + + return dict(dependencies) + + +def declared_dependencies(pyproject_path: Path) -> dict[str, set[str]]: + with pyproject_path.open("rb") as file: + config = tomllib.load(file) + + project_dependencies = config.get("project", {}).get("dependencies", []) + dynamic_dependencies = ( + config.get("tool", {}) + .get("hatch", {}) + .get("metadata", {}) + .get("hooks", {}) + .get("uv-dynamic-versioning", {}) + .get("dependencies", []) + ) + dependencies = requirement_dependencies(project_dependencies) + for name, extras in requirement_dependencies(dynamic_dependencies, allow_version_template=True).items(): + dependencies.setdefault(name, set()).update(extras) + return dependencies + + +def imported_modules(source_root: Path, repository_root: Path) -> dict[str, list[str]]: + imports: dict[str, set[str]] = defaultdict(set) + for path in sorted(source_root.rglob("*.py")): + tree = ast.parse(path.read_text(), filename=str(path)) + relative_path = str(path.relative_to(repository_root)) + # Include guarded and TYPE_CHECKING imports; the recipe re-verifies runtime use before fixing a gap. + for node in ast.walk(tree): + if isinstance(node, ast.Import): + modules = [alias.name.partition(".")[0] for alias in node.names] + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + modules = [node.module.partition(".")[0]] + else: + continue + for module in modules: + if module not in sys.stdlib_module_names and module != "data_designer": + imports[module].add(relative_path) + return {module: sorted(paths) for module, paths in sorted(imports.items())} + + +def resolve_distribution( + module: str, + candidates: list[str], + declared: set[str], + declared_anywhere: set[str], +) -> str | None: + if override := MODULE_DISTRIBUTION_OVERRIDES.get(module): + return normalize_name(override) + + normalized = sorted({normalize_name(candidate) for candidate in candidates}) + for pool in (declared, declared_anywhere): + matches = [candidate for candidate in normalized if candidate in pool] + if len(matches) == 1: + return matches[0] + return normalized[0] if len(normalized) == 1 else None + + +def installed_requirements(distribution_name: str, selected_extras: set[str]) -> dict[str, set[str]]: + try: + requirements = distribution(distribution_name).requires or [] + except PackageNotFoundError: + return {} + + return requirement_dependencies(requirements, selected_extras) + + +def audit_repository( + repository_root: Path, + module_distributions: dict[str, list[str]] | None = None, + requirement_map: dict[str, list[str]] | None = None, +) -> dict[str, Any]: + repository_root = repository_root.resolve() + package_dirs = sorted(path.parent for path in repository_root.glob("packages/*/pyproject.toml")) + projects: dict[str, dict[str, Any]] = {} + + for package_dir in package_dirs: + with (package_dir / "pyproject.toml").open("rb") as file: + project_name = normalize_name(tomllib.load(file)["project"]["name"]) + projects[project_name] = { + "path": str(package_dir.relative_to(repository_root)), + "declarations": declared_dependencies(package_dir / "pyproject.toml"), + "imports": imported_modules(package_dir / "src", repository_root), + } + + declared_by: dict[str, set[str]] = defaultdict(set) + for project_name, project in projects.items(): + for dependency in project["declarations"]: + declared_by[dependency].add(project_name) + + distribution_map = module_distributions if module_distributions is not None else packages_distributions() + declared_anywhere = set(declared_by) + + requirement_cache: dict[tuple[str, frozenset[str]], dict[str, set[str]]] = {} + + def requirements_for(distribution_name: str, selected_extras: set[str]) -> dict[str, set[str]]: + if distribution_name in projects: + return projects[distribution_name]["declarations"] + if requirement_map is not None: + return requirement_dependencies(requirement_map.get(distribution_name, []), selected_extras) + cache_key = (distribution_name, frozenset(selected_extras)) + if cache_key not in requirement_cache: + requirement_cache[cache_key] = installed_requirements(distribution_name, selected_extras) + return requirement_cache[cache_key] + + def dependency_closure(distribution_name: str, selected_extras: set[str]) -> set[str]: + closure = set() + pending = [(distribution_name, selected_extras)] + visited: set[tuple[str, frozenset[str]]] = set() + while pending: + current, extras = pending.pop() + current_key = (current, frozenset(extras)) + if current_key in visited: + continue + visited.add(current_key) + for dependency, dependency_extras in requirements_for(current, extras).items(): + closure.add(dependency) + pending.append((dependency, dependency_extras)) + return closure + + results = [] + for project_name, project in projects.items(): + declarations = project["declarations"] + declared = set(declarations) + dependency_closures = { + dependency: dependency_closure(dependency, extras) for dependency, extras in declarations.items() + } + resolved_imports: dict[str, dict[str, set[str]]] = defaultdict(lambda: {"modules": set(), "files": set()}) + unresolved = [] + + for module, files in project["imports"].items(): + distribution_name = resolve_distribution( + module, + distribution_map.get(module, []), + declared, + declared_anywhere, + ) + if distribution_name is None: + unresolved.append({"module": module, "files": files}) + continue + resolved_imports[distribution_name]["modules"].add(module) + resolved_imports[distribution_name]["files"].update(files) + + missing = [] + for distribution_name, usage in sorted(resolved_imports.items()): + if distribution_name in declared: + continue + sibling_declarations = sorted(declared_by[distribution_name] - {project_name}) + guaranteed_by = sorted( + dependency for dependency, closure in dependency_closures.items() if distribution_name in closure + ) + missing.append( + { + "dependency": distribution_name, + "modules": sorted(usage["modules"]), + "files": sorted(usage["files"]), + "declared_by": sibling_declarations, + "guaranteed_by": guaranteed_by, + "severity": "low" if guaranteed_by else "high", + } + ) + + results.append( + { + "package": project_name, + "path": project["path"], + "declared": sorted(declared), + "imported": sorted(resolved_imports), + "missing": missing, + "unresolved_modules": unresolved, + } + ) + + return {"packages": results} + + +def main() -> None: + parser = argparse.ArgumentParser(description="Inventory package import/dependency gaps") + parser.add_argument("--root", type=Path, default=Path.cwd()) + parser.add_argument("--output", type=Path) + args = parser.parse_args() + + result = audit_repository(args.root) + payload = json.dumps(result, indent=2) + "\n" + if args.output: + args.output.write_text(payload) + else: + print(payload, end="") + + +if __name__ == "__main__": + main() diff --git a/scripts/manage_agentic_ci_attempts.py b/scripts/manage_agentic_ci_attempts.py new file mode 100644 index 000000000..a59afd5d5 --- /dev/null +++ b/scripts/manage_agentic_ci_attempts.py @@ -0,0 +1,99 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import argparse +import json +import os +from pathlib import Path +from typing import Any + +MARKERS = ( + "gate_violation", + "lockfile_checkout_failed", + "lockfile_missing", + "lockfile_not_committed", + "lockfile_verification_failed", +) + + +def new_open_attempt_groups(state_path: Path, prior_path: Path) -> list[dict[str, Any]]: + state = json.loads(state_path.read_text()) + prior = json.loads(prior_path.read_text()) + prior_counts = {entry["id"]: entry["n"] for entry in prior} + groups: dict[tuple[int | None, str | None], dict[str, Any]] = {} + + for entry in state.get("attempted_fixes") or []: + attempts = entry.get("attempts") or [] + if not attempts or attempts[-1].get("outcome") != "open": + continue + if len(attempts) <= prior_counts.get(entry["id"], 0): + continue + + attempt = attempts[-1] + key = (attempt.get("pr_number"), attempt.get("branch")) + group = groups.setdefault( + key, + { + "pr_number": attempt.get("pr_number"), + "branch": attempt.get("branch"), + "finding_ids": [], + }, + ) + group["finding_ids"].append(entry["id"]) + + return list(groups.values()) + + +def abandon_open_attempts(state_path: Path, finding_ids: list[str], marker: str) -> None: + if marker not in MARKERS: + raise ValueError(f"Unsupported marker: {marker}") + + state = json.loads(state_path.read_text()) + selected = set(finding_ids) + updated = set() + for entry in state.get("attempted_fixes") or []: + if entry.get("id") not in selected: + continue + attempts = entry.get("attempts") or [] + if attempts and attempts[-1].get("outcome") == "open": + attempts[-1]["outcome"] = "abandoned" + attempts[-1][marker] = True + updated.add(entry["id"]) + + if updated != selected: + missing = ", ".join(sorted(selected - updated)) + raise ValueError(f"Open attempts not found: {missing}") + + temporary_path = state_path.with_suffix(f"{state_path.suffix}.tmp") + temporary_path.write_text(json.dumps(state, indent=2)) + os.replace(temporary_path, state_path) + + +def main() -> None: + parser = argparse.ArgumentParser(description="Manage agentic CI attempted-fix groups") + subparsers = parser.add_subparsers(dest="command", required=True) + + groups_parser = subparsers.add_parser("groups") + groups_parser.add_argument("--state", type=Path, required=True) + groups_parser.add_argument("--prior", type=Path, required=True) + + abandon_parser = subparsers.add_parser("abandon") + abandon_parser.add_argument("--state", type=Path, required=True) + abandon_parser.add_argument("--finding-ids", required=True) + abandon_parser.add_argument("--marker", choices=MARKERS, required=True) + + args = parser.parse_args() + if args.command == "groups": + print(json.dumps(new_open_attempt_groups(args.state, args.prior), separators=(",", ":"))) + return + + finding_ids = json.loads(args.finding_ids) + if not isinstance(finding_ids, list) or not all(isinstance(value, str) for value in finding_ids): + parser.error("--finding-ids must be a JSON list of strings") + abandon_open_attempts(args.state, finding_ids, args.marker) + + +if __name__ == "__main__": + main()