diff --git a/CLAUDE.md b/CLAUDE.md index 0ce19af..84e3994 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,7 +175,7 @@ Status legend: **ACTIVE** (current spec/contract, AI agents should read first) / | `docs/exit_codes.md` | ACTIVE | Stable CLI exit code policy (0 / 1 / 2 / 3 / 4) for CI integration, including `--strict-repair`, `severity: info` Advisor channel, and per-subcommand notes | | `docs/json_schema.md` | ACTIVE | CLI JSON envelopes — verdict / compile at `schema_version="6"`, compile-repair at independent `schema_version="1"`, validate-plan at independent `schema_version="2"`. Includes compatibility policy and v2→v3 through v5→v6 diffs (plus validate-plan v1→v2) | | `docs/cli_test_inventory.md` | ACTIVE | CLI test coverage inventory, runtime notes, and conservative reduction candidates | -| `docs/target_yaml_guide.md` | ACTIVE | `target.yaml` authoring guide. Practical companion to `design.md §4` / `cli_usage.md`. Centralises authoring hazards D1 (`--package-root` scope vs `tests/` visibility), D3 (template / user constraint duplication), D4 (config-only PR の vacuous PASS) — 2026-05-07 Session 4 dogfooding 由来 | +| `docs/target_yaml_guide.md` | ACTIVE | `target.yaml` authoring guide. Practical companion to `design.md §4` / `cli_usage.md`. Centralises authoring hazards D1 (`--package-root` scope vs `tests/` visibility), D3 (template / user constraint duplication), D4 (config-only PR の vacuous PASS), D6 (nested-function complexity 盲点) — 2026-05-07 Session 4 / 2026-05-28 real-PR dogfooding 由来 | | `docs/target_authoring_surface.md` | ACTIVE | Authoring surface 設計契約 (Brief 8 / CSCI-41)。target.yaml は hand-written 必須でない / 生成経路 3 通り (recipe + sources / catalog 参照 / hand-written) / LLM 経路は Brief 8b 分離 / 全経路は verdict 前に declared intent として固定 / Authoring・Advisor・Provenance surface は evaluator 不可参照 / `candidate_code_used: false` 固定。§23.3.1 の実装側 catch-up | | `docs/ssp_protocol.md` | ACTIVE | SSP v0.1 normative spec: SensorOutput / Finding / SSPDelta / SSPVerdict definitions, 5-element SAST + 3-element SCA fingerprint, Python profile AST normalization, delta computation, verdict precedence (`unknown > fail > pass`), JSON Schema artifact, Sensor Provenance Invariant (§23.1 mirror), determinism requirements, core isolation contract | | `docs/ssp_usage_guide.md` | ACTIVE | SSP practical usage guide: quick start (Semgrep SAST / pip-audit SCA / fixture mode), output formats (JSON / human / SARIF), CI integration (GitHub Actions workflow / exit code routing / fixture-based CI), hand-built SensorOutput examples, delta computation overview, relationship to core Semantic CI | diff --git a/docs/cli_usage.md b/docs/cli_usage.md index 94e480e..51a1f31 100644 --- a/docs/cli_usage.md +++ b/docs/cli_usage.md @@ -738,7 +738,7 @@ semantic-ci target-doctor [--target ] [--package-root ] [--format {human,json}] [--output ] ``` -Audits a `target.yaml` for seven authoring hazards and renders them as +Audits a `target.yaml` for eight authoring hazards and renders them as advisories. Advisor surface — advisory presence does not change the verdict and does not change the exit code (`docs/exit_codes.md`). @@ -747,6 +747,7 @@ verdict and does not change the exit code (`docs/exit_codes.md`). | `ADVISORY-D1` | `test_surface_delta.*` constraint exists, but no test files (`test_*.py` / `*_test.py` / `tests/`) are visible under `--package-root`. | | `ADVISORY-D3` | A user constraint duplicates a template-expanded constraint (same kind/target/operator/expected). | | `ADVISORY-D4` | The target is lock-only and the candidate diff (`--baseline-rev` ↔ `--candidate-rev`) touches no Python files; the verdict would be a vacuous PASS. Skipped silently when neither rev is given and git is unavailable. | +| `ADVISORY-D6` | The target declares a `complexity_delta` constraint and the candidate diff (`--baseline-rev` ↔ `--candidate-rev`) grows the nested-function count in an in-scope Python file. The complexity extractor does not descend into nested defs, so a reported cyclomatic/cognitive decrease may be displacement, not simplification. Skipped silently when neither rev is given and git is unavailable. | | `ADVISORY-I1` | `intent` is the empty string. Repair adapters and `validate-plan` produce better guidance when intent describes the change purpose; use `init --intent` or edit `target.yaml`. | | `ADVISORY-P1` | `primary_kind: feature` has no positive addition constraint. | | `ADVISORY-P2` | `primary_kind: bugfix` has no `test_surface_delta.new_cases` expectation. | diff --git a/docs/dogfooding_findings_tracker.md b/docs/dogfooding_findings_tracker.md index 24e0dbd..24c3a40 100644 --- a/docs/dogfooding_findings_tracker.md +++ b/docs/dogfooding_findings_tracker.md @@ -22,9 +22,9 @@ Status taxonomy: | D3 | 2026-05-07 Session 4 dogfood | template / user constraint duplication | user constraint shadows template default with identical semantics, doubles evidence | **解決** | `docs/target_yaml_guide.md` Hazard 2 + `ADVISORY-D3` detector | | D4 | 2026-05-07 Session 4 dogfood | vacuous PASS (out-of-scope diff) | config-only PR produces empty Python `CodeStateDelta`; lock-only template constraints pass without exercising the actual change | **解決** | `docs/target_yaml_guide.md` Hazard 3 + `ADVISORY-D4` detector | | D5 | 2026-05-07 Session 5 dogfood (FINDING-1) | set operator partial-match semantics | partial-dict `expected` items canonicalised as different elements from full extractor records — false positive on `includes_*` / `subset_of` / `superset_of`, **false negative (CI bypass) on `excludes_all`** | **解決** | PR #65 (CSCI-35c) — Match Schema partial-record matching + flat-projection aliases + `evidence.matched`; schema_version v4→v5 | -| D6 | 2026-05-28 real-PR complexity dogfood (FINDING-F1) | vacuous PASS (extractor coverage gap) — **重複・関連 = sibling of D4** | nested function bodies are excluded from `ComplexityEntry` by `python_complexity_extractor` spec (`api_surface` parity); refactor that nests outer-function body into nested helpers reports large CC drop while real complexity is unchanged | **未解決** | Candidate paths: (a) `docs/target_yaml_guide.md` new Hazard 4 + `ADVISORY-D6` detector mirroring D4; (b) extractor spec change to emit nested-function entries (long-term, schema-impacting). Reproduction: langgraph PR #3700 (8/1 vacuous PASS in real-PR pass) | +| D6 | 2026-05-28 real-PR complexity dogfood (FINDING-F1) | vacuous PASS (extractor coverage gap) — **重複・関連 = sibling of D4** | nested function bodies are excluded from `ComplexityEntry` by `python_complexity_extractor` spec (`api_surface` parity); refactor that nests outer-function body into nested helpers reports large CC drop while real complexity is unchanged | **解決** | `docs/target_yaml_guide.md` Hazard 4 + `ADVISORY-D6` detector (`authoring/hazards.py::detect_d6`, candidate path (a), mirrors D4's diff-aware contract) — fires when a verdict-participating `complexity_delta` constraint meets nested-def growth in the in-scope candidate diff (`--baseline-rev` ↔ `--candidate-rev`); growth signal computed per file via `authoring/nested_defs.py`, syntax-error sides skipped fail-silent. Path (b) (extractor emits nested-function entries) remains a long-term, schema-impacting option, not pursued. Reproduction: langgraph PR #3700 (8/1 vacuous PASS in real-PR pass) | | D7 | 2026-05-28 real-PR complexity dogfood (FINDING-F2) | authoring mismatch (operator / constraint pairing) | `extract-method` refactor is mathematically guaranteed to **micro-increase cyclomatic** (each extracted function adds base 1), even with `_` prefix discipline and api_surface preserved. Cognitive is the metric that drops. Authors declaring `complexity_delta.cyclomatic ≤ 0` for extract-method refactors hit a structural false-FAIL | **未解決** | Candidate paths: (a) authoring guide section "Choosing complexity metric per refactor pattern" recommending `cognitive_delta` for extract-method; (b) future `ADVISORY-D7` detector emitted when a `change.primary_kind=refactor` target uses `cyclomatic_delta ≤ 0` and the diff matches extract-method shape. Low priority: this is authoring advice, not a CI integrity hazard | -| D8 | 2026-06-07 scale + security dogfood (SCA gap) | SCA sensor dependency-source discovery gap | SSP SCA auto-discovery (`_requirements_file` in `src/semantic_ci_code/cli/commands/ssp.py`) only found `requirements.txt` at repo root; the `--locked` fallback only accepted `pylock.toml` / requirements lockfiles. PEP 621 pyproject-only projects (litellm) and `pdm.lock` projects (pdm) declared deps in unrecognised formats → `pip-audit --locked .` errors "no lockfiles found" → empty JSON → adapter degraded to `unknown` (exit 3). Correct graceful degradation (no silent false PASS, honours `unknown > fail > pass`) but a real usability gap that blocked SCA on most modern Python projects | **解決** | CSCI-55 — dependency source discovery now recognises `requirements.txt`, `pylock.toml` / `pylock.*.toml`, `uv.lock`, `pdm.lock`, `poetry.lock`, and static PEP 621 `[project].dependencies`; lock sources are converted deterministically to pinned temp requirements, optional/non-default-group/marker-inactive packages are filtered, and malformed recognized sources fail closed to SSP `unknown` | +| D8 | 2026-06-07 scale + security dogfood (SCA gap) | SCA sensor dependency-source discovery gap | SSP SCA auto-discovery (`_requirements_file` in `src/semantic_ci_code/cli/commands/ssp.py`) only found `requirements.txt` at repo root; the `--locked` fallback only accepted `pylock.toml` / requirements lockfiles. PEP 621 pyproject-only projects (litellm) and `pdm.lock` projects (pdm) declared deps in unrecognised formats → `pip-audit --locked .` errors "no lockfiles found" → empty JSON → adapter degraded to `unknown` (exit 3). Correct graceful degradation (no silent false PASS, honours `unknown > fail > pass`) but a real usability gap that blocked SCA on most modern Python projects | **解決** | CSCI-55 / PR #151 — dependency source discovery now recognises `requirements.txt`, `pylock.toml` / `pylock.*.toml`, `uv.lock`, `pdm.lock`, `poetry.lock`, and static PEP 621 `[project].dependencies`; lock sources are converted deterministically to pinned temp requirements, optional/non-default-group/marker-inactive packages are filtered, and malformed recognized sources fail closed to SSP `unknown` | ## Reading order @@ -37,8 +37,8 @@ Status taxonomy: ## Classification at a glance - **重複・関連 pairs**: D4 ↔ D6 (both are "vacuous PASS" via extractor coverage gap, distinct mechanism — D4 is "diff outside Python scope", D6 is "diff inside scope but inside nested function") -- **解決 (6 of 8)**: D1, D2, D3, D4, D5, D8 -- **未解決 (2 of 8)**: D6 (mitigation path open), D7 (authoring advice, low priority) +- **解決 (7 of 8)**: D1, D2, D3, D4, D5, D6, D8 +- **未解決 (1 of 8)**: D7 (authoring advice, low priority) - **observation-only (not a D#)**: F6 (pattern-SAST logic-vuln blindspot) — **UNTESTED HYPOTHESIS, not a demonstrated observation in the 2026-06-07 pass**: the Semgrep registry rulesets returned HTTP 403, so Semgrep ran with 0 loaded rules over 0 paths and produced no valid SAST measurement. F6 records the *a-priori* expectation that deterministic SAST misses semantic / business-logic vulns, cross-linked to Phase H (`docs/llm_sensor_adapter_planning.md`) as **motivation** — it is **not** empirically validated by this pass. Recorded in `docs/dogfooding_scale_and_security.md` (which now carries a validity warning + repro note for redoing the SAST sub-pass under a network policy allowing `semgrep.dev`). Distinct from the demonstrated observations of the same pass: real vulns merged-then-fixed (git evidence) and SCA clean-on-litellm (pip-audit positive-controlled with `jinja2==2.11.2` → 5 CVEs) ## Source pass index diff --git a/docs/exit_codes.md b/docs/exit_codes.md index 769ee19..a022182 100644 --- a/docs/exit_codes.md +++ b/docs/exit_codes.md @@ -78,8 +78,8 @@ git errors (`CompileError` on `target.yaml`, git revision resolution failure when `--baseline-rev` / `--candidate-rev` is given, git unavailable when explicitly required) exit 3. Internal bugs exit 4. When neither `--baseline-rev` nor `--candidate-rev` is given and git is -unavailable or no baseline can be resolved, ADVISORY-D4 is silently -skipped rather than failing. There is no `--strict-advice` flag; CI that +unavailable or no baseline can be resolved, ADVISORY-D4 and ADVISORY-D6 +are silently skipped rather than failing. There is no `--strict-advice` flag; CI that wants to gate on advisory presence should consume `--format json` and apply a workflow-level policy. Silent success on bad input is forbidden — the advisor surface only suppresses the verdict step, not the input diff --git a/docs/json_schema.md b/docs/json_schema.md index aa1015c..7200518 100644 --- a/docs/json_schema.md +++ b/docs/json_schema.md @@ -318,12 +318,12 @@ compile-repair, or validate-plan envelopes. The shape is pinned by |---|---| | `schema_version` | Always `"advisory-1"`. | | `subcommand` | Always `"target-doctor"`. | -| `advisories[].code` | One of `ADVISORY-D1`, `ADVISORY-D3`, `ADVISORY-D4`, `ADVISORY-I1`, `ADVISORY-P1`, `ADVISORY-P2`, `ADVISORY-S1`. | +| `advisories[].code` | One of `ADVISORY-D1`, `ADVISORY-D3`, `ADVISORY-D4`, `ADVISORY-D6`, `ADVISORY-I1`, `ADVISORY-P1`, `ADVISORY-P2`, `ADVISORY-S1`. | | `advisories[].severity` | Always `"info"` — the Advisor surface never participates in the verdict (`docs/code_semantic_ci_design.md §23.3.1`). | | `advisories[].message` | Human-readable explanation of the hazard. | | `advisories[].evidence` | Per-advisory diagnostic fields (e.g. `constraint_id`, `target`, `package_root`, `files_touched_count`). | -Advisories are emitted in canonical order (D1 → D3 → D4 → I1 → P1 → P2 → S1) +Advisories are emitted in canonical order (D1 → D3 → D4 → D6 → I1 → P1 → P2 → S1) with `constraint_id` as the within-code tiebreak so output is byte-identical across runs. Advisory presence does not change the exit code — see `docs/exit_codes.md`. diff --git a/docs/target_yaml_guide.md b/docs/target_yaml_guide.md index 253936a..8b695d7 100644 --- a/docs/target_yaml_guide.md +++ b/docs/target_yaml_guide.md @@ -281,6 +281,41 @@ non-Python artifacts (see `design.md §13` on out-of-scope items). It is `semantic-ci target-doctor` (Brief 8 / CSCI-43) detects this hazard as `ADVISORY-D4` once it lands. +## Hazard 4 — Nested functions are invisible to complexity numbers (D6) + +`python_complexity_extractor` only emits entries for the `api_surface` +parity subset: module-level functions and direct methods of module-level +classes. When it walks a function body it **stops descent at nested +`def` boundaries** — a nested helper contributes 0 to the enclosing +function's cyclomatic/cognitive number, and the helper itself is never +emitted as its own entry. + +The consequence for `complexity_delta` constraints is the D6 sibling of +Hazard 3's vacuous PASS (observed on a real PR in +`docs/dogfooding_real_pr_complexity.md` FINDING-F1): a refactor that moves +an outer function's body into function-nested helpers reports a large +complexity *drop* while the real complexity is unchanged — it has only +been displaced below the extractor's horizon. A lock like +`complexity_delta.cyclomatic less_than_or_equal 0` passes, and the verdict +silently endorses the displacement. + +**What this means for authors:** + +- A green complexity verdict on a refactor that introduced nested helpers + is weaker evidence than the number suggests. Check whether the helpers + should be **module-level functions** (possibly `_`-prefixed) — those are + extracted, complexity-counted, and constrained. +- This is the extractor behaving as specified (`api_surface` emission + parity, `design.md` CSCI-8), not a counting bug. The blind spot is the + declared trade-off for a deterministic, owned formula. + +`semantic-ci target-doctor` detects this hazard as `ADVISORY-D6` when +`--baseline-rev` / `--candidate-rev` are given: it fires when the target +declares a verdict-participating `complexity_delta` constraint and the +candidate diff grows the nested-def count in an in-scope Python file. +Growth is a heuristic displacement signal, not proof — review the diff +rather than treating the advisory as a verdict. + ## Constraint Authoring Tips ### Pick `kind` deliberately diff --git a/src/semantic_ci_code/authoring/advisory.py b/src/semantic_ci_code/authoring/advisory.py index df9be64..871dacf 100644 --- a/src/semantic_ci_code/authoring/advisory.py +++ b/src/semantic_ci_code/authoring/advisory.py @@ -7,6 +7,7 @@ "ADVISORY-D1", "ADVISORY-D3", "ADVISORY-D4", + "ADVISORY-D6", "ADVISORY-I1", "ADVISORY-P1", "ADVISORY-P2", diff --git a/src/semantic_ci_code/authoring/hazards.py b/src/semantic_ci_code/authoring/hazards.py index 8657594..ee480ef 100644 --- a/src/semantic_ci_code/authoring/hazards.py +++ b/src/semantic_ci_code/authoring/hazards.py @@ -1,10 +1,11 @@ """Authoring hazard detectors for `semantic-ci target-doctor`. Each `detect_*` function inspects a `CompiledTarget` (and optional -package-root or files-touched context) and returns zero or one `Advisory`. -The combined entrypoint `detect_advisories` returns a deterministically -ordered tuple. All detectors are pure: no network, no LLM, no I/O outside -of reading `package_root` for D1. +package-root, files-touched, or nested-def-growth context) and returns zero +or one `Advisory`. The combined entrypoint `detect_advisories` returns a +deterministically ordered tuple. All detectors are pure: no network, no +LLM, no I/O outside of reading `package_root` for D1 — diff-derived +context (D4 / D6) is computed by the CLI layer and passed in. The advisor surface never participates in the verdict (`docs/code_semantic_ci_design.md §23.3.1`); detection or non-detection @@ -17,6 +18,7 @@ from pathlib import Path from semantic_ci_code.authoring.advisory import Advisory +from semantic_ci_code.authoring.nested_defs import NestedDefGrowth from semantic_ci_code.compiler.target_compiler import ( CompiledConstraint, CompiledTarget, @@ -35,6 +37,7 @@ "ADVISORY-D1", "ADVISORY-D3", "ADVISORY-D4", + "ADVISORY-D6", "ADVISORY-I1", "ADVISORY-P1", "ADVISORY-P2", @@ -95,12 +98,14 @@ def detect_advisories( *, package_root: Path | None = None, files_touched: tuple[Path, ...] | None = None, + nested_def_growth: tuple[NestedDefGrowth, ...] | None = None, ) -> tuple[Advisory, ...]: """Run every detector and return the advisories in canonical order. `package_root` is required for D1; when omitted the detector is - skipped silently. `files_touched` is required for D4; when omitted - (e.g. git not available) the detector is skipped. + skipped silently. `files_touched` is required for D4 and + `nested_def_growth` for D6; when omitted (e.g. git not available) + the corresponding detector is skipped. """ advisories: list[Advisory] = [] if package_root is not None: @@ -108,6 +113,8 @@ def detect_advisories( advisories.extend(detect_d3(target)) if files_touched is not None: advisories.extend(detect_d4(target, files_touched=files_touched)) + if nested_def_growth is not None: + advisories.extend(detect_d6(target, nested_def_growth=nested_def_growth)) advisories.extend(detect_i1(target)) advisories.extend(detect_p1(target)) advisories.extend(detect_p2(target)) @@ -220,6 +227,72 @@ def detect_d4( ) +def detect_d6( + target: CompiledTarget, + *, + nested_def_growth: tuple[NestedDefGrowth, ...], +) -> tuple[Advisory, ...]: + """Complexity constraint + nested-def growth = displaced complexity. + + `python_complexity_extractor` stops descent at nested def boundaries + (`api_surface` emission parity), so a refactor that moves an outer + function's body into function-nested helpers reports a large + cyclomatic/cognitive drop while the real complexity is unchanged — + the lock passes and the verdict silently endorses the displacement + (D6, sibling of D4's vacuous PASS). The detector fires when the + target declares a verdict-participating `complexity_delta` constraint + AND the candidate diff grows the nested-def count in at least one + in-scope Python file. Growth is a heuristic displacement signal, not + proof — the advisory asks for review, never seats the verdict. + + An empty `nested_def_growth` tuple means "diff inspected, no Python + files with parseable content on both sides"; `None` (inapplicable, + git unavailable) is filtered out by the caller before reaching this + detector — mirroring D4's `files_touched` contract. + """ + complexity_constraints = tuple( + c + for c in target.constraints + if _is_complexity_delta_target(c.target) and _participates_in_verdict(c) + ) + if not complexity_constraints: + return () + grown = tuple(g for g in nested_def_growth if g.candidate_count > g.baseline_count) + if not grown: + return () + total_added = sum(g.candidate_count - g.baseline_count for g in grown) + constraint_ids = ", ".join(repr(c.id) for c in complexity_constraints) + return ( + Advisory( + code="ADVISORY-D6", + message=( + f"the candidate diff adds {total_added} nested function " + f"definition(s) across {len(grown)} file(s), and the target " + f"declares complexity_delta constraint(s) ({constraint_ids}). " + f"The complexity extractor does not descend into nested " + f"functions, so complexity moved into nested helpers vanishes " + f"from cyclomatic/cognitive numbers — a reported decrease may " + f"be displacement, not simplification. Review whether the new " + f"nested helpers should be module-level functions (which are " + f"extracted and constrained) instead." + ), + evidence={ + "constraint_ids": [c.id for c in complexity_constraints], + "nested_defs_added": total_added, + "grown_files_count": len(grown), + "files": [ + { + "path": g.path, + "baseline_nested_defs": g.baseline_count, + "candidate_nested_defs": g.candidate_count, + } + for g in grown[:5] + ], + }, + ), + ) + + def detect_i1(target: CompiledTarget) -> tuple[Advisory, ...]: """Empty intent degrades repair adapter and validate-plan guidance.""" if target.intent: @@ -681,6 +754,17 @@ def _is_open_path(path: str) -> bool: return head in _OPEN_PATH_PREFIXES +def _is_complexity_delta_target(path: str) -> bool: + """Match `complexity_delta` and any `complexity_delta.` path. + + Both whole-delta and leaf (`.cyclomatic` / `.cognitive`) targets are + deceived equally when complexity is displaced into nested functions, + so D6 scopes on the path head alone. + """ + head = path.split(".", 1)[0] + return head == "complexity_delta" + + def _targets_new_test_cases(constraint: CompiledConstraint) -> bool: if "test_surface_delta" not in constraint.target: return False diff --git a/src/semantic_ci_code/authoring/nested_defs.py b/src/semantic_ci_code/authoring/nested_defs.py new file mode 100644 index 0000000..ba1e12d --- /dev/null +++ b/src/semantic_ci_code/authoring/nested_defs.py @@ -0,0 +1,60 @@ +"""Nested-function counting for ADVISORY-D6 (nested-function blind spot). + +`python_complexity_extractor` stops descent at nested ``FunctionDef`` / +``AsyncFunctionDef`` / ``ClassDef`` boundaries and only emits entries for +the ``api_surface`` parity subset (module-level defs + direct methods of +module-level classes). Complexity moved into function-nested helpers +therefore disappears from ``complexity_delta`` numbers (D6, +`docs/dogfooding_findings_tracker.md`). This module provides the +deterministic per-file signal — the count of function-nested defs — that +the CLI layer feeds to `detect_d6` as `NestedDefGrowth` records. + +Scope note: only defs nested (at any depth) inside another function are +counted. Lambdas are not counted — they cannot contain statements, so they +cannot absorb branch complexity the way a nested def can. Methods of +module-level classes are extractor-visible and not counted. +""" + +from __future__ import annotations + +import ast +from dataclasses import dataclass + + +@dataclass(frozen=True) +class NestedDefGrowth: + """Per-file nested-def counts on both sides of the candidate diff. + + `path` is the candidate-side repo-relative posix path. A file absent + on one side contributes a count of 0 for that side. + """ + + path: str + baseline_count: int + candidate_count: int + + +def count_nested_defs(source: str) -> int | None: + """Count function defs nested inside another function. + + Returns `None` when `source` does not parse — the caller must skip + the file rather than treat it as zero, so a syntax error cannot + fabricate or suppress a growth signal. + """ + try: + tree = ast.parse(source) + except (SyntaxError, ValueError): + return None + count = 0 + # Iterative walk: (node, inside_function). A def found while inside a + # function counts, including defs inside a class that is itself inside + # a function (the extractor emits neither). + stack: list[tuple[ast.AST, bool]] = [(tree, False)] + while stack: + node, inside_function = stack.pop() + for child in ast.iter_child_nodes(node): + is_def = isinstance(child, ast.FunctionDef | ast.AsyncFunctionDef) + if is_def and inside_function: + count += 1 + stack.append((child, inside_function or is_def)) + return count diff --git a/src/semantic_ci_code/cli/commands/target_doctor.py b/src/semantic_ci_code/cli/commands/target_doctor.py index b03d4a5..2aadee2 100644 --- a/src/semantic_ci_code/cli/commands/target_doctor.py +++ b/src/semantic_ci_code/cli/commands/target_doctor.py @@ -1,19 +1,21 @@ """`semantic-ci target-doctor` — Advisor surface command. -Renders authoring hazards (D1 / D3 / D4 / P1 / P2 / S1) detected on a -`target.yaml`. Advisor surface (`docs/code_semantic_ci_design.md §23.3.1`): -the verdict is not computed and advisory presence does not change the exit -code. Usage / configuration errors return 2; engine / git failures return 3; -unhandled exceptions return 4 (`docs/exit_codes.md`). +Renders authoring hazards (D1 / D3 / D4 / D6 / I1 / P1 / P2 / S1) detected +on a `target.yaml`. Advisor surface (`docs/code_semantic_ci_design.md +§23.3.1`): the verdict is not computed and advisory presence does not change +the exit code. Usage / configuration errors return 2; engine / git failures +return 3; unhandled exceptions return 4 (`docs/exit_codes.md`). """ from __future__ import annotations import json from argparse import Namespace +from dataclasses import dataclass from pathlib import Path from semantic_ci_code.authoring import detect_advisories +from semantic_ci_code.authoring.nested_defs import NestedDefGrowth, count_nested_defs from semantic_ci_code.cli.command_support import ( engine_error, internal_bug, @@ -25,6 +27,7 @@ from semantic_ci_code.cli.git_runtime import ( GitError, GitNotFoundError, + blob_text, is_git_available, repo_root, resolve_baseline, @@ -45,11 +48,12 @@ def run_target_doctor(args: Namespace) -> int: target_path = discover_target(args.target, cwd=Path.cwd()) compiled = load_compiled_target(target_path) package_root = resolve_package_root(args.package_root) - files_touched = _resolve_files_touched(args, package_root=package_root) + diff_context = _resolve_diff_context(args, package_root=package_root) advisories = detect_advisories( compiled, package_root=package_root, - files_touched=files_touched, + files_touched=diff_context.files_touched if diff_context else None, + nested_def_growth=diff_context.nested_def_growth if diff_context else None, ) payload = build_doctor_payload(advisories) if args.format == "json": @@ -71,24 +75,39 @@ def run_target_doctor(args: Namespace) -> int: return internal_bug(exc, args) -def _resolve_files_touched( +@dataclass(frozen=True) +class _DiffContext: + """Candidate-diff context shared by the diff-aware detectors. + + `files_touched` feeds D4 (vacuous PASS on non-Python diffs); + `nested_def_growth` feeds D6 (complexity displaced into nested + functions). Both are derived from the same numstat pass so the two + detectors always describe the same baseline ↔ candidate range. + """ + + files_touched: tuple[Path, ...] + nested_def_growth: tuple[NestedDefGrowth, ...] + + +def _resolve_diff_context( args: Namespace, *, package_root: Path, -) -> tuple[Path, ...] | None: - """Resolve the candidate diff file list for D4, filtered to the +) -> _DiffContext | None: + """Resolve the candidate diff for D4 / D6, filtered to the `--package-root` slice. `semantic-ci check` extracts only inside `--package-root`, so D4's "vacuous PASS" hazard applies whenever the in-scope slice has no Python diff — even if other parts of the repo do. We filter the repo-wide numstat to paths under `package_root` before - classification. + classification. D6 reads the in-scope Python entries' content at + both revs to compute the nested-def growth signal. When the user passes `--baseline-rev` or `--candidate-rev`, git failures surface as exit 3. When neither is passed and git is - unavailable / no repo / no baseline, D4 is silently skipped (returns - `None`). + unavailable / no repo / no baseline, both detectors are silently + skipped (returns `None`). """ explicit = args.baseline_rev is not None or args.candidate_rev is not None @@ -124,30 +143,73 @@ def _resolve_files_touched( raise return None - # Include both sides of rename entries so D4 can see Python touches - # on either side. A `foo.py -> bar.txt` rename has new path - # `bar.txt` and `old_path=foo.py`; dropping `old_path` would let D4 - # misclassify the diff as non-Python and emit a vacuous-pass - # warning even though the validator would extract a Python delta - # (api_surface_delta.removed_public on the old name). - paths: list[Path] = [] - for entry in entries: - paths.append(entry.path) - if entry.old_path is not None: - paths.append(entry.old_path) - # Filter to the in-scope slice: numstat paths are repo-relative, # so we resolve under `root` and check whether they land inside # `package_root`. Diffs outside package_root are extracted as # nothing by `semantic-ci check`, so they cannot prevent a # vacuous PASS on the in-scope slice. package_root_resolved = package_root.resolve() - filtered: list[Path] = [] - for path in paths: + + def in_scope(path: Path) -> bool: full = (root / path).resolve() try: - if full.is_relative_to(package_root_resolved): - filtered.append(path) + return full.is_relative_to(package_root_resolved) except (OSError, ValueError): + return False + + # Include both sides of rename entries so D4 can see Python touches + # on either side. A `foo.py -> bar.txt` rename has new path + # `bar.txt` and `old_path=foo.py`; dropping `old_path` would let D4 + # misclassify the diff as non-Python and emit a vacuous-pass + # warning even though the validator would extract a Python delta + # (api_surface_delta.removed_public on the old name). + files_touched: list[Path] = [] + growth: list[NestedDefGrowth] = [] + for entry in entries: + candidate_path = entry.path + baseline_path = entry.old_path if entry.old_path is not None else entry.path + if in_scope(candidate_path): + files_touched.append(candidate_path) + if entry.old_path is not None and in_scope(entry.old_path): + files_touched.append(entry.old_path) + + # D6 signal: nested-def counts on both sides of every in-scope + # Python entry. A file absent at one rev (added / deleted) + # contributes 0 on that side, and so does an out-of-scope side: + # `semantic-ci check --package-root` never observed it, so a + # rename across the package-root boundary must count as 0 → N + # (newly in-scope nested defs) rather than N → N (Codex review + # P2 — matching counts would mask the displacement). A side that + # fails to parse skips the entry entirely so a syntax error + # cannot fabricate or suppress a growth signal. + if candidate_path.suffix.lower() != ".py" and baseline_path.suffix.lower() != ".py": + continue + if not (in_scope(candidate_path) or in_scope(baseline_path)): + continue + baseline_count = ( + _nested_defs_at(root, baseline_ref, baseline_path) if in_scope(baseline_path) else 0 + ) + candidate_count = ( + _nested_defs_at(root, candidate_ref, candidate_path) if in_scope(candidate_path) else 0 + ) + if baseline_count is None or candidate_count is None: continue - return tuple(filtered) + growth.append( + NestedDefGrowth( + path=candidate_path.as_posix(), + baseline_count=baseline_count, + candidate_count=candidate_count, + ) + ) + return _DiffContext(files_touched=tuple(files_touched), nested_def_growth=tuple(growth)) + + +def _nested_defs_at(root: Path, ref: str, path: Path) -> int | None: + """Nested-def count for `path` at `ref`; 0 when absent, None on + syntax error (caller skips the entry).""" + if path.suffix.lower() != ".py": + return 0 + source = blob_text(ref, path.as_posix(), cwd=root) + if source is None: + return 0 + return count_nested_defs(source) diff --git a/src/semantic_ci_code/cli/git_runtime.py b/src/semantic_ci_code/cli/git_runtime.py index aa23095..5a0ea46 100644 --- a/src/semantic_ci_code/cli/git_runtime.py +++ b/src/semantic_ci_code/cli/git_runtime.py @@ -142,6 +142,20 @@ def repo_root(cwd: Path) -> Path: return Path(output.strip()).resolve() +def blob_text(ref: str, path: str, *, cwd: Path) -> str | None: + """Return the file content at `ref:path`, or None when absent. + + `path` must be repo-relative posix form. The ref is validated; a + missing path at the ref (added/deleted file on one diff side) is the + expected miss and maps to None rather than an error. + """ + ensure_safe_ref(ref) + try: + return run_git(["show", f"{ref}:{path}"], cwd=cwd) + except GitCommandError: + return None + + def tree_object_id(ref: str, subpath: str, *, cwd: Path) -> str: ensure_safe_ref(ref) spec = f"{ref}^{{tree}}" if subpath == "." else f"{ref}:{subpath}" diff --git a/src/semantic_ci_code/cli/main.py b/src/semantic_ci_code/cli/main.py index c65e6f1..9dc3d81 100644 --- a/src/semantic_ci_code/cli/main.py +++ b/src/semantic_ci_code/cli/main.py @@ -338,12 +338,12 @@ def build_parser() -> argparse.ArgumentParser: target_doctor.add_argument( "--baseline-rev", default=None, - help="baseline git ref for ADVISORY-D4 (config-only diff vacuous PASS)", + help="baseline git ref for ADVISORY-D4 / D6 (diff-aware hazards)", ) target_doctor.add_argument( "--candidate-rev", default=None, - help="candidate git ref for ADVISORY-D4; defaults to HEAD", + help="candidate git ref for ADVISORY-D4 / D6; defaults to HEAD", ) target_doctor.add_argument( "--format", diff --git a/src/semantic_ci_code/schemas/doctor_advisory.schema.json b/src/semantic_ci_code/schemas/doctor_advisory.schema.json index f5237ce..09adcda 100644 --- a/src/semantic_ci_code/schemas/doctor_advisory.schema.json +++ b/src/semantic_ci_code/schemas/doctor_advisory.schema.json @@ -20,6 +20,7 @@ "ADVISORY-D1", "ADVISORY-D3", "ADVISORY-D4", + "ADVISORY-D6", "ADVISORY-I1", "ADVISORY-P1", "ADVISORY-P2", diff --git a/tests/authoring/test_hazards.py b/tests/authoring/test_hazards.py index cbcfa84..591f831 100644 --- a/tests/authoring/test_hazards.py +++ b/tests/authoring/test_hazards.py @@ -3,6 +3,7 @@ from pathlib import Path from semantic_ci_code.authoring.hazards import detect_advisories +from semantic_ci_code.authoring.nested_defs import NestedDefGrowth from semantic_ci_code.compiler.target_compiler import compile_target_svp @@ -49,3 +50,162 @@ def test_advisory_order_i1_between_d4_and_p1(): assert "ADVISORY-I1" in codes assert "ADVISORY-P1" in codes assert codes.index("ADVISORY-D4") < codes.index("ADVISORY-I1") < codes.index("ADVISORY-P1") + + +# --------------------------------------------------------------------------- +# ADVISORY-D6 — complexity constraint + nested-def growth = displacement +# --------------------------------------------------------------------------- + +_COMPLEXITY_TARGET = ( + "intent: simplify hot path\n" + "change:\n" + " primary_kind: refactor\n" + "constraints:\n" + " - id: cc_lock\n" + " kind: delta\n" + " target: complexity_delta.cyclomatic\n" + " operator: less_than_or_equal\n" + " expected: 0\n" +) + + +def _growth(path: str = "src/mod.py", baseline: int = 0, candidate: int = 2) -> NestedDefGrowth: + return NestedDefGrowth(path=path, baseline_count=baseline, candidate_count=candidate) + + +def test_detect_d6_fires_on_complexity_constraint_with_growth(): + target = _compile_target(_COMPLEXITY_TARGET) + + advisories = detect_advisories(target, nested_def_growth=(_growth(),)) + d6 = [advisory for advisory in advisories if advisory.code == "ADVISORY-D6"] + + assert len(d6) == 1 + assert "nested" in d6[0].message + assert d6[0].evidence["constraint_ids"] == ["cc_lock"] + assert d6[0].evidence["nested_defs_added"] == 2 + assert d6[0].evidence["grown_files_count"] == 1 + assert d6[0].evidence["files"] == [ + {"path": "src/mod.py", "baseline_nested_defs": 0, "candidate_nested_defs": 2} + ] + + +def test_detect_d6_fires_on_cognitive_leaf_and_whole_mapping(): + yaml_text = ( + "intent: simplify\n" + "change:\n" + " primary_kind: refactor\n" + "constraints:\n" + " - id: cognitive_lock\n" + " kind: delta\n" + " target: complexity_delta.cognitive\n" + " operator: less_than_or_equal\n" + " expected: 0\n" + ) + target = _compile_target(yaml_text) + + advisories = detect_advisories(target, nested_def_growth=(_growth(),)) + d6 = [advisory for advisory in advisories if advisory.code == "ADVISORY-D6"] + + assert len(d6) == 1 + assert d6[0].evidence["constraint_ids"] == ["cognitive_lock"] + + +def test_detect_d6_silent_without_complexity_constraint(): + target = _compile_target( + "intent: refactor\nchange:\n primary_kind: refactor\nconstraints: []\n" + ) + + advisories = detect_advisories(target, nested_def_growth=(_growth(),)) + + assert "ADVISORY-D6" not in [advisory.code for advisory in advisories] + + +def test_detect_d6_silent_without_growth(): + target = _compile_target(_COMPLEXITY_TARGET) + + advisories = detect_advisories( + target, + nested_def_growth=( + _growth(baseline=2, candidate=2), + _growth(path="src/other.py", baseline=3, candidate=1), + ), + ) + + assert "ADVISORY-D6" not in [advisory.code for advisory in advisories] + + +def test_detect_d6_silent_on_empty_growth_tuple(): + target = _compile_target(_COMPLEXITY_TARGET) + + advisories = detect_advisories(target, nested_def_growth=()) + + assert "ADVISORY-D6" not in [advisory.code for advisory in advisories] + + +def test_detect_d6_skipped_when_context_is_none(): + target = _compile_target(_COMPLEXITY_TARGET) + + advisories = detect_advisories(target, nested_def_growth=None) + + assert "ADVISORY-D6" not in [advisory.code for advisory in advisories] + + +def test_detect_d6_ignores_non_verdict_participating_constraint(): + yaml_text = ( + "intent: simplify\n" + "change:\n" + " primary_kind: refactor\n" + "constraints:\n" + " - id: cc_info\n" + " kind: delta\n" + " target: complexity_delta.cyclomatic\n" + " operator: less_than_or_equal\n" + " expected: 0\n" + " severity: info\n" + " unknown_policy: ignore\n" + ) + target = _compile_target(yaml_text) + + advisories = detect_advisories(target, nested_def_growth=(_growth(),)) + + assert "ADVISORY-D6" not in [advisory.code for advisory in advisories] + + +def test_detect_d6_evidence_truncates_files_to_five(): + target = _compile_target(_COMPLEXITY_TARGET) + growth = tuple(_growth(path=f"src/mod_{i}.py") for i in range(7)) + + advisories = detect_advisories(target, nested_def_growth=growth) + d6 = [advisory for advisory in advisories if advisory.code == "ADVISORY-D6"] + + assert len(d6) == 1 + assert d6[0].evidence["grown_files_count"] == 7 + assert d6[0].evidence["nested_defs_added"] == 14 + assert len(d6[0].evidence["files"]) == 5 + + +def test_advisory_order_d6_between_d4_and_i1(): + yaml_text = ( + 'intent: ""\n' + "change:\n" + " primary_kind: refactor\n" + "constraints:\n" + " - id: cc_lock\n" + " kind: delta\n" + " target: complexity_delta.cyclomatic\n" + " operator: less_than_or_equal\n" + " expected: 0\n" + ) + target = _compile_target(yaml_text) + + advisories = detect_advisories( + target, + files_touched=(Path("README.md"),), + nested_def_growth=(_growth(),), + ) + codes = [advisory.code for advisory in advisories] + + assert "ADVISORY-D4" in codes + assert "ADVISORY-D6" in codes + assert "ADVISORY-I1" in codes + assert codes.index("ADVISORY-D4") < codes.index("ADVISORY-D6") < codes.index("ADVISORY-I1") diff --git a/tests/authoring/test_nested_defs.py b/tests/authoring/test_nested_defs.py new file mode 100644 index 0000000..6e5970c --- /dev/null +++ b/tests/authoring/test_nested_defs.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +from semantic_ci_code.authoring.nested_defs import NestedDefGrowth, count_nested_defs + + +def test_module_level_defs_are_not_nested(): + source = "def foo(): return 1\n\nasync def bar(): return 2\n" + assert count_nested_defs(source) == 0 + + +def test_methods_of_module_level_class_are_not_nested(): + source = "class A:\n def method(self): return 1\n" + assert count_nested_defs(source) == 0 + + +def test_def_inside_function_counts(): + source = "def outer():\n def helper(): return 1\n return helper()\n" + assert count_nested_defs(source) == 1 + + +def test_async_def_inside_function_counts(): + source = "def outer():\n async def helper(): return 1\n return helper\n" + assert count_nested_defs(source) == 1 + + +def test_deeply_nested_defs_count_each_level(): + source = ( + "def outer():\n" + " def middle():\n" + " def inner(): return 1\n" + " return inner()\n" + " return middle()\n" + ) + assert count_nested_defs(source) == 2 + + +def test_method_of_class_inside_function_counts(): + # Neither the class nor its method is extractor-visible + # (api_surface parity: only module-level classes' direct methods). + source = ( + "def outer():\n class Local:\n def method(self): return 1\n return Local\n" + ) + assert count_nested_defs(source) == 1 + + +def test_def_inside_method_counts(): + source = ( + "class A:\n def method(self):\n def helper(): return 1\n return helper()\n" + ) + assert count_nested_defs(source) == 1 + + +def test_lambda_does_not_count(): + source = "def outer():\n return lambda x: x + 1\n" + assert count_nested_defs(source) == 0 + + +def test_def_inside_module_scope_compound_statement_is_not_nested(): + # The extractor emits defs inside module-scope compound statements + # (e.g. `if TYPE_CHECKING:`), so they are not displacement targets. + source = "if True:\n def foo(): return 1\n" + assert count_nested_defs(source) == 0 + + +def test_syntax_error_returns_none(): + assert count_nested_defs("def broken(:\n") is None + + +def test_empty_source_counts_zero(): + assert count_nested_defs("") == 0 + + +def test_growth_record_is_frozen_and_comparable(): + growth = NestedDefGrowth(path="src/mod.py", baseline_count=0, candidate_count=2) + assert growth.candidate_count > growth.baseline_count diff --git a/tests/cli/test_target_doctor.py b/tests/cli/test_target_doctor.py index 971ec98..7da0531 100644 --- a/tests/cli/test_target_doctor.py +++ b/tests/cli/test_target_doctor.py @@ -1,6 +1,6 @@ """CSCI-43: `semantic-ci target-doctor` (Advisor surface). -Covers all six advisories (D1 / D3 / D4 / P1 / P2 / S1) with both a +Covers the advisories (D1 / D3 / D4 / D6 / I1 / P1 / P2 / S1) with both a detection fixture and a false-positive prevention fixture, the JSON envelope schema, the exit-code contract from `docs/exit_codes.md` and `docs/brief_8_planning.md §6.3.3`, and the determinism + argparse spec @@ -1575,3 +1575,212 @@ def test_d4_cli_silent_when_python_diff_present(tmp_path: Path): assert result.returncode == 0 payload = parse_json(result.stdout) assert "ADVISORY-D4" not in [a["code"] for a in payload["advisories"]] + + +# --------------------------------------------------------------------------- +# CLI integration with git for D6 (covers nested_def_growth resolution path) +# --------------------------------------------------------------------------- + +_TARGET_COMPLEXITY_LOCK = ( + "intent: simplify hot path\n" + "change:\n" + " primary_kind: refactor\n" + "constraints:\n" + " - id: cc_lock\n" + " kind: delta\n" + " target: complexity_delta.cyclomatic\n" + " operator: less_than_or_equal\n" + " expected: 0\n" +) + +_FLAT_FUNCTION = ( + "def process(items):\n" + " out = []\n" + " for item in items:\n" + " if item:\n" + " out.append(item)\n" + " return out\n" +) + +# Same behaviour, body displaced into a nested helper: the extractor's +# cyclomatic for `process` drops while real complexity is unchanged. +_NESTED_REFACTOR = ( + "def process(items):\n" + " def _collect():\n" + " out = []\n" + " for item in items:\n" + " if item:\n" + " out.append(item)\n" + " return out\n" + " return _collect()\n" +) + + +def _d6_repo(tmp_path: Path, *, candidate_source: str, target_yaml: str) -> tuple[Path, str]: + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-b", "main") + _git(repo, "config", "user.name", "tester") + _git(repo, "config", "user.email", "t@t") + (repo / "src").mkdir() + (repo / "src" / "mod.py").write_text(_FLAT_FUNCTION, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "baseline") + baseline_sha = _git_head(repo) + _git(repo, "checkout", "-b", "feature") + (repo / "src" / "mod.py").write_text(candidate_source, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "refactor") + _write_target(repo / "target.yaml", target_yaml) + return repo, baseline_sha + + +def _run_doctor_json(repo: Path, baseline_sha: str) -> dict: + result = run_semantic_ci( + repo, + "target-doctor", + "--target", + str(repo / "target.yaml"), + "--package-root", + "src", + "--baseline-rev", + baseline_sha, + "--candidate-rev", + "HEAD", + "--format", + "json", + ) + assert result.returncode == 0, result.stderr + return parse_json(result.stdout) + + +def test_d6_cli_fires_on_nested_def_displacement(tmp_path: Path): + repo, baseline_sha = _d6_repo( + tmp_path, + candidate_source=_NESTED_REFACTOR, + target_yaml=_TARGET_COMPLEXITY_LOCK, + ) + + payload = _run_doctor_json(repo, baseline_sha) + jsonschema.validate(payload, DOCTOR_SCHEMA) + d6 = [a for a in payload["advisories"] if a["code"] == "ADVISORY-D6"] + + assert len(d6) == 1 + assert d6[0]["evidence"]["constraint_ids"] == ["cc_lock"] + assert d6[0]["evidence"]["nested_defs_added"] == 1 + assert d6[0]["evidence"]["files"] == [ + {"path": "src/mod.py", "baseline_nested_defs": 0, "candidate_nested_defs": 1} + ] + + +def test_d6_cli_silent_without_complexity_constraint(tmp_path: Path): + repo, baseline_sha = _d6_repo( + tmp_path, + candidate_source=_NESTED_REFACTOR, + target_yaml="intent: refactor\nchange:\n primary_kind: refactor\nconstraints: []\n", + ) + + payload = _run_doctor_json(repo, baseline_sha) + codes = [a["code"] for a in payload["advisories"]] + + assert "ADVISORY-D6" not in codes + # The diff touches a Python file, so D4 must stay silent too. + assert "ADVISORY-D4" not in codes + + +def test_d6_cli_silent_without_nested_growth(tmp_path: Path): + repo, baseline_sha = _d6_repo( + tmp_path, + candidate_source=_FLAT_FUNCTION.replace("if item:", "if item is not None:"), + target_yaml=_TARGET_COMPLEXITY_LOCK, + ) + + payload = _run_doctor_json(repo, baseline_sha) + + assert "ADVISORY-D6" not in [a["code"] for a in payload["advisories"]] + + +def test_d6_cli_skips_unparseable_candidate_file(tmp_path: Path): + repo, baseline_sha = _d6_repo( + tmp_path, + candidate_source="def broken(:\n", + target_yaml=_TARGET_COMPLEXITY_LOCK, + ) + + payload = _run_doctor_json(repo, baseline_sha) + + assert "ADVISORY-D6" not in [a["code"] for a in payload["advisories"]] + + +def test_d6_cli_counts_new_file_from_zero_baseline(tmp_path: Path): + repo, baseline_sha = _d6_repo( + tmp_path, + candidate_source=_FLAT_FUNCTION + "\n# touched\n", + target_yaml=_TARGET_COMPLEXITY_LOCK, + ) + (repo / "src" / "helpers.py").write_text(_NESTED_REFACTOR, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "add helper module with nested def") + + payload = _run_doctor_json(repo, baseline_sha) + d6 = [a for a in payload["advisories"] if a["code"] == "ADVISORY-D6"] + + assert len(d6) == 1 + assert d6[0]["evidence"]["files"] == [ + {"path": "src/helpers.py", "baseline_nested_defs": 0, "candidate_nested_defs": 1} + ] + + +def test_d6_cli_fires_on_rename_into_package_root_scope(tmp_path: Path): + """Codex review P2: a rename across the --package-root boundary must + count the out-of-scope side as 0 (the extractor never observed it). + `lib/helpers.py` (1 nested def) renamed to `src/helpers.py` is 0 -> 1 + for the src slice, not 1 -> 1, so D6 must fire.""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-b", "main") + _git(repo, "config", "user.name", "tester") + _git(repo, "config", "user.email", "t@t") + (repo / "src").mkdir() + (repo / "lib").mkdir() + (repo / "src" / "mod.py").write_text(_FLAT_FUNCTION, encoding="utf-8") + (repo / "lib" / "helpers.py").write_text(_NESTED_REFACTOR, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "baseline") + baseline_sha = _git_head(repo) + _git(repo, "checkout", "-b", "feature") + _git(repo, "mv", "lib/helpers.py", "src/helpers.py") + _git(repo, "commit", "-m", "move helpers into package root") + _write_target(repo / "target.yaml", _TARGET_COMPLEXITY_LOCK) + + payload = _run_doctor_json(repo, baseline_sha) + d6 = [a for a in payload["advisories"] if a["code"] == "ADVISORY-D6"] + + assert len(d6) == 1 + assert d6[0]["evidence"]["files"] == [ + {"path": "src/helpers.py", "baseline_nested_defs": 0, "candidate_nested_defs": 1} + ] + + +def test_d6_cli_silent_on_rename_out_of_package_root_scope(tmp_path: Path): + """Mirror case: moving a nested-def file out of the package root is a + 0-growth event for the src slice (candidate side counts 0).""" + repo = tmp_path / "repo" + repo.mkdir() + _git(repo, "init", "-b", "main") + _git(repo, "config", "user.name", "tester") + _git(repo, "config", "user.email", "t@t") + (repo / "src").mkdir() + (repo / "src" / "helpers.py").write_text(_NESTED_REFACTOR, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "baseline") + baseline_sha = _git_head(repo) + _git(repo, "checkout", "-b", "feature") + (repo / "lib").mkdir() + _git(repo, "mv", "src/helpers.py", "lib/helpers.py") + _git(repo, "commit", "-m", "move helpers out of package root") + _write_target(repo / "target.yaml", _TARGET_COMPLEXITY_LOCK) + + payload = _run_doctor_json(repo, baseline_sha) + + assert "ADVISORY-D6" not in [a["code"] for a in payload["advisories"]]