diff --git a/CHANGELOG.md b/CHANGELOG.md index 616030e..7ab2cf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,69 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/). +## [Unreleased] + +### managing-generators + +- New rule `python-relationship-references` covering the three + accepted forms for relationship fields in + `self.client.create()` (HFID dict, ID dict, SDK object) plus + the bare-string and over-packed-list anti-patterns. +- New rule `python-multi-peer-add` explaining that + `RelationshipManager.add()` takes one peer per call; iterate + the peer list. +- New rule `patterns-natural-key-preflight` covering form-driven + mutations that collide with bootstrap-seeded keys. +- New advisory rule `testing-integration` recommending + end-to-end runs against a live instance before merge. +- Fixed misleading `device_type_id` example in `python-generate` + that suggested bare-string relationship values. +- Added 5 deterministic evals + 4 graders to `eval.yaml` (first + evals for this skill). + +### managing-transforms + +- New rule `queries-union-fragments` documenting the need for + GraphQL inline fragments on union-typed relationships + (`DcimDevice.location`, `Organization*` peers) to avoid + "Cannot query field 'X' on type 'Y'" failures in + CoreRepository schema-sync. +- New rule `artifacts-async-regen-polling` documenting that + `POST /api/artifact/generate` is fire-and-forget and + programmatic callers must poll `CoreArtifact` to confirm + completion. +- Added 2 deterministic evals + graders to `eval.yaml` (first + evals for this skill). + +### Hook (session-start) + +- Added per-path skill activation triggers to the + SessionStart hook's `additionalContext`. When the user + touches `generators/**`, `transforms/**`, `queries/**`, + `checks/**`, `objects/**`, `menus/**`, or `.gql` files, the + hook now explicitly tells Claude to read the corresponding + skill (with specific rule files) before editing. The trigger + block opens with a rationale — integration-layer rules live + in the skills, not in source, so pattern-matching produces + bugs that pass unit tests but fail at runtime. + +### infrahub-common + +- New rule `deployment-gql-dry-run` recommending that any PR + touching `queries/**/*.gql` runs `infrahubctl render + --dry-run` (or the equivalent check/generator run) against + a live Infrahub schema before merge. YAML schema validation + and Python type checking don't catch GraphQL query/schema + mismatches; only a live dry-run does. This is the failure + class that hides behind silent CoreRepository + schema-sync hangs. + +### Notes + +- `skillgrade --smoke` was not run in this branch (skillgrade + CLI not installed in the dev environment). Run before merging + to baseline pass rates against the new evals. + ## [1.1.0] - 2026-03-18 ### Added diff --git a/dev/plans/2026-05-18-managing-generators-integration-rules.md b/dev/plans/2026-05-18-managing-generators-integration-rules.md new file mode 100644 index 0000000..564dfe2 --- /dev/null +++ b/dev/plans/2026-05-18-managing-generators-integration-rules.md @@ -0,0 +1,2510 @@ +# managing-generators Integration-Layer Rules Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. +> Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the integration-layer gap in +`infrahub-managing-generators` that allowed four real-world bugs +(HFID over-pack, bare-string relationship, multi-peer add as +list, uniqueness collision) to consume hours during a demo +session. + +**Architecture:** Four new rule files + one rule edit + five eval +tasks gated by AST-based deterministic graders. New +`graders/managing-generators/` package built from the +`graders/managing-schemas/` template, with Python source parsing +via the stdlib `ast` module replacing the existing YAML-only +flow. + +**Tech Stack:** Markdown (rules + spec), Python 3.10+ stdlib +`ast` module (graders), PyYAML (eval task config), skillgrade +(eval runner), pytest (grader unit tests). + +**Spec:** +[`dev/specs/2026-05-18-managing-generators-integration-rules-design.md`](../specs/2026-05-18-managing-generators-integration-rules-design.md) + +--- + +## File Structure + +### New files + +```text +skills/infrahub-managing-generators/rules/ +├── python-relationship-references.md # new rule (CRITICAL) +├── python-multi-peer-add.md # new rule (HIGH) +├── patterns-natural-key-preflight.md # new rule (MEDIUM) +└── testing-integration.md # new rule (LOW, advisory) + +graders/managing-generators/ +├── __init__.py # empty package marker +├── lib.py # AST helpers + CHECKS + run_checks +├── check_relationship_hfid_encoding.py +├── check_relationship_three_forms.py +├── check_multi_peer_iteration.py +└── check_natural_key_preflight.py + +tests/graders/ +└── test_generators_lib.py # pytest tests for the new lib.py +``` + +### Modified files + +```text +skills/infrahub-managing-generators/rules/ +├── python-generate.md # fix misleading example +└── _sections.md # note new rules + +eval.yaml # add 5 tasks +evaluations/managing-generators.json # regenerated by sync-evals.py +CHANGELOG.md # add release-notes bullets +``` + +--- + +## Task 1: Set up grader package skeleton + +**Files:** + +- Create: `graders/managing-generators/__init__.py` +- Create: `graders/managing-generators/lib.py` + +The package skeleton matches `graders/managing-schemas/`. We start +with empty/minimal contents so subsequent tasks can layer on AST +helpers, check functions, and the `run_checks` entry point. + +- [ ] **Step 1: Create the package marker** + +Create empty file: + +```bash +mkdir -p graders/managing-generators +touch graders/managing-generators/__init__.py +``` + +- [ ] **Step 2: Create lib.py skeleton** + +Create `graders/managing-generators/lib.py` with module docstring, +imports, and an empty `CHECKS` registry. The full helper set is +added in Task 2. + +```python +"""Shared grader library for infrahub-managing-generators evaluations. + +Provides Python AST parsing helpers, individual assertion check +functions, a CHECKS registry, and the top-level ``run_checks`` +function that returns skillgrade JSON format. + +Graders for this skill parse Python source (the generator class +the model produced as ``output.py``) rather than YAML. Some +expressions cannot be resolved from AST alone — variable +references, function-call results — and the check functions +treat those as "indeterminate" (passing) rather than failing. +The checks fail only on shapes that are demonstrably wrong: +bare string literals where a dict reference is required, +over-packed list literals, list literals passed to ``.add()``. + +Usage (in a per-task grader script):: + + from pathlib import Path + from lib import run_checks + + result = run_checks( + ["relationship-hfid-form-correct", "no-bare-string-relationship"], + Path("output.py"), + ) + print(result) +""" + +from __future__ import annotations + +import ast +import json +from pathlib import Path +from typing import Any, Iterator + + +# --------------------------------------------------------------------------- +# I/O helpers +# --------------------------------------------------------------------------- + + +def load_output_py(path: Path) -> tuple[ast.Module | None, str]: + """Load a Python source file and return ``(parsed_tree, raw_text)``. + + Returns ``(None, "")`` if the file does not exist. + Returns ``(None, raw)`` if the file exists but has a syntax error. + """ + try: + raw = Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return None, "" + try: + tree = ast.parse(raw) + except SyntaxError: + return None, raw + return tree, raw + + +# --------------------------------------------------------------------------- +# CHECKS registry (populated by later tasks) +# --------------------------------------------------------------------------- + +CHECKS: dict[str, Any] = {} + + +# --------------------------------------------------------------------------- +# run_checks — top-level entry point for grader scripts +# --------------------------------------------------------------------------- + + +def run_checks( + check_names: list[str], + output_path: Path, +) -> dict: + """Run named checks against a Python source file. + + Returns skillgrade JSON: ``{"score": float, "details": str, + "checks": [...]}``. + + Raises ``KeyError`` if any name in ``check_names`` is not in + ``CHECKS``. + """ + tree, raw = load_output_py(output_path) + + entries: list[dict] = [] + passed_count = 0 + + for name in check_names: + fn = CHECKS[name] # raises KeyError for unknown names + try: + ok, msg = fn(tree, raw_text=raw) + except Exception as exc: # defensive — never let one check crash all + ok, msg = False, f"Error running check: {exc}" + + if ok: + passed_count += 1 + entries.append({"name": name, "passed": ok, "message": msg}) + + total = len(check_names) + score = round(passed_count / total, 4) if total > 0 else 0.0 + + failed_names = [e["name"] for e in entries if not e["passed"]] + if failed_names: + details = f"{passed_count}/{total} checks passed. Failed: {', '.join(failed_names)}" + else: + details = f"All {total} checks passed." + + return {"score": score, "details": details, "checks": entries} +``` + +- [ ] **Step 3: Smoke-test the skeleton imports cleanly** + +Run from repo root: + +```bash +python -c " +import importlib.util +from pathlib import Path +p = Path('graders/managing-generators/lib.py') +spec = importlib.util.spec_from_file_location('m', p) +m = importlib.util.module_from_spec(spec) +spec.loader.exec_module(m) +print('CHECKS:', m.CHECKS) +print('run_checks empty result:', m.run_checks([], Path('/nonexistent.py'))) +" +``` + +Expected output: + +```text +CHECKS: {} +run_checks empty result: {'score': 0.0, 'details': 'All 0 checks passed.', 'checks': []} +``` + +- [ ] **Step 4: Commit** + +```bash +git add graders/managing-generators/__init__.py graders/managing-generators/lib.py +git commit -m "feat(graders): scaffold managing-generators grader package" +``` + +--- + +## Task 2: Add AST helper functions + +**Files:** + +- Modify: `graders/managing-generators/lib.py` +- Create: `tests/graders/test_generators_lib.py` + +These helpers walk the AST to identify the call shapes the +graders care about. We test them in isolation before any check +function uses them. + +- [ ] **Step 1: Write the failing test scaffold** + +Create `tests/graders/test_generators_lib.py`: + +```python +"""Tests for graders/managing-generators/lib.py AST helpers.""" + +import importlib.util +from pathlib import Path + +import pytest + +# Load the hyphenated-directory module by file path +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_LIB_PATH = _REPO_ROOT / "graders" / "managing-generators" / "lib.py" +_spec = importlib.util.spec_from_file_location( + "managing_generators_graders_lib", _LIB_PATH +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + +find_client_create_calls = _mod.find_client_create_calls +find_client_get_calls = _mod.find_client_get_calls +find_relationship_add_calls = _mod.find_relationship_add_calls +get_data_dict_items = _mod.get_data_dict_items +is_hfid_dict = _mod.is_hfid_dict +is_id_dict = _mod.is_id_dict +is_bare_string = _mod.is_bare_string +is_name_or_attribute = _mod.is_name_or_attribute +load_output_py = _mod.load_output_py + + +import ast + + +def _parse(src: str) -> ast.Module: + return ast.parse(src) + + +def test_find_client_create_calls_finds_await_self_client_create(): + tree = _parse( + "async def f():\n" + " await self.client.create(kind='D', data={'name': 'x'})\n" + ) + calls = find_client_create_calls(tree) + assert len(calls) == 1 + + +def test_find_client_create_calls_ignores_other_create_calls(): + tree = _parse( + "async def f():\n" + " await some.other.create(kind='D')\n" + " await self.client.update(kind='D')\n" + ) + assert find_client_create_calls(tree) == [] + + +def test_get_data_dict_items_returns_literal_dict_entries(): + tree = _parse( + "x = self.client.create(kind='D', data={'name': 'x', 'status': 'active'})\n" + ) + calls = [n for n in ast.walk(tree) if isinstance(n, ast.Call)] + items = get_data_dict_items(calls[0]) + assert set(items.keys()) == {"name", "status"} + + +def test_is_hfid_dict_matches_hfid_list_literal(): + node = ast.parse("x = {'hfid': ['cEdge-1000']}").body[0].value + ok, elts = is_hfid_dict(node) + assert ok is True + assert len(elts) == 1 + assert isinstance(elts[0], ast.Constant) + assert elts[0].value == "cEdge-1000" + + +def test_is_hfid_dict_rejects_non_dict_or_wrong_key(): + assert is_hfid_dict(ast.parse("x = 'foo'").body[0].value) == (False, None) + assert is_hfid_dict(ast.parse("x = {'id': '...'}").body[0].value) == (False, None) + + +def test_is_id_dict_matches_id_dict_literal(): + assert is_id_dict(ast.parse("x = {'id': 'abc'}").body[0].value) is True + assert is_id_dict(ast.parse("x = {'hfid': ['a']}").body[0].value) is False + + +def test_is_bare_string_matches_only_string_constants(): + assert is_bare_string(ast.parse("x = 'hi'").body[0].value) is True + assert is_bare_string(ast.parse("x = 42").body[0].value) is False + assert is_bare_string(ast.parse("x = ['a']").body[0].value) is False + + +def test_is_name_or_attribute_matches_variable_and_attribute_refs(): + assert is_name_or_attribute(ast.parse("x = device").body[0].value) is True + assert is_name_or_attribute(ast.parse("x = self.device").body[0].value) is True + assert is_name_or_attribute(ast.parse("x = 'd'").body[0].value) is False + + +def test_find_relationship_add_calls_finds_any_add(): + tree = _parse( + "def f():\n" + " group.members.add(device)\n" + " other.foo.add([1, 2])\n" + " plain.bar()\n" + ) + assert len(find_relationship_add_calls(tree)) == 2 + + +def test_find_client_get_calls_finds_self_client_get(): + tree = _parse( + "async def f():\n" + " x = await self.client.get(kind='D', name__value='x')\n" + " y = await self.client.create(kind='D')\n" + ) + assert len(find_client_get_calls(tree)) == 1 + + +def test_load_output_py_returns_none_for_missing_file(): + tree, raw = load_output_py(Path("/nonexistent/path.py")) + assert tree is None + assert raw == "" + + +def test_load_output_py_returns_none_tree_on_syntax_error(tmp_path): + p = tmp_path / "broken.py" + p.write_text("def f(:\n") + tree, raw = load_output_py(p) + assert tree is None + assert "def f" in raw +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: + +```bash +pytest tests/graders/test_generators_lib.py -v +``` + +Expected: all tests fail with `AttributeError` (helpers +not yet defined on the module). + +- [ ] **Step 3: Add helpers to lib.py** + +In `graders/managing-generators/lib.py`, between the I/O helpers +and the `CHECKS` registry, insert: + +```python +# --------------------------------------------------------------------------- +# AST helpers +# --------------------------------------------------------------------------- + + +def _iter_calls(tree: ast.Module) -> Iterator[ast.Call]: + """Yield every ``ast.Call`` node anywhere in the tree.""" + for node in ast.walk(tree): + if isinstance(node, ast.Call): + yield node + + +def _is_self_client_method(call: ast.Call, method: str) -> bool: + """True if ``call.func`` is ``self.client.``.""" + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != method: + return False + if not isinstance(func.value, ast.Attribute) or func.value.attr != "client": + return False + if not isinstance(func.value.value, ast.Name) or func.value.value.id != "self": + return False + return True + + +def find_client_create_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``self.client.create(...)`` call sites.""" + if tree is None: + return [] + return [c for c in _iter_calls(tree) if _is_self_client_method(c, "create")] + + +def find_client_get_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``self.client.get(...)`` call sites.""" + if tree is None: + return [] + return [c for c in _iter_calls(tree) if _is_self_client_method(c, "get")] + + +def find_relationship_add_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``.add(...)`` call sites. + + Does not filter by attribute path (``.members``, ``.peers``, etc.) — + the check functions decide whether to narrow further. + """ + if tree is None: + return [] + return [ + c for c in _iter_calls(tree) + if isinstance(c.func, ast.Attribute) and c.func.attr == "add" + ] + + +def get_kwarg(call: ast.Call, name: str) -> ast.AST | None: + """Return the value node for keyword argument ``name``, or None.""" + for kw in call.keywords: + if kw.arg == name: + return kw.value + return None + + +def get_data_dict_items(call: ast.Call) -> dict[str, ast.AST]: + """If ``data=`` is a dict literal, return ``{key_str: value_ast}``. + + Returns ``{}`` if ``data`` is missing or not a literal dict. + Non-string keys in the dict literal are skipped silently. + """ + data = get_kwarg(call, "data") + if not isinstance(data, ast.Dict): + return {} + items: dict[str, ast.AST] = {} + for k, v in zip(data.keys, data.values): + if isinstance(k, ast.Constant) and isinstance(k.value, str): + items[k.value] = v + return items + + +def is_hfid_dict(node: ast.AST) -> tuple[bool, list[ast.AST] | None]: + """True if ``node`` is ``{"hfid": [...]}`` with a list value. + + Returns ``(True, [element_ast_nodes])`` on match, else + ``(False, None)``. + """ + if not isinstance(node, ast.Dict): + return False, None + for k, v in zip(node.keys, node.values): + if isinstance(k, ast.Constant) and k.value == "hfid": + if isinstance(v, ast.List): + return True, list(v.elts) + return False, None + return False, None + + +def is_id_dict(node: ast.AST) -> bool: + """True if ``node`` is ``{"id": }``.""" + if not isinstance(node, ast.Dict): + return False + for k in node.keys: + if isinstance(k, ast.Constant) and k.value == "id": + return True + return False + + +def is_bare_string(node: ast.AST) -> bool: + """True if ``node`` is a bare string constant ``ast.Constant(value=str)``.""" + return isinstance(node, ast.Constant) and isinstance(node.value, str) + + +def is_name_or_attribute(node: ast.AST) -> bool: + """True if ``node`` is a Name or Attribute reference (likely an SDK obj).""" + return isinstance(node, (ast.Name, ast.Attribute)) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +pytest tests/graders/test_generators_lib.py -v +``` + +Expected: all 11 tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add graders/managing-generators/lib.py tests/graders/test_generators_lib.py +git commit -m "feat(graders): add AST helpers for managing-generators lib" +``` + +--- + +## Task 3: Bug 2/3 — HFID encoding (rule + grader + eval) + +**Files:** + +- Create: `skills/infrahub-managing-generators/rules/python-relationship-references.md` +- Modify: `graders/managing-generators/lib.py` (add checks) +- Modify: `tests/graders/test_generators_lib.py` (add check tests) +- Create: `graders/managing-generators/check_relationship_hfid_encoding.py` +- Modify: `eval.yaml` + +Covers bugs 2 (over-packed HFID list for single-component target) +and 3 (bare string interpreted as id). + +- [ ] **Step 1: Write the rule file** + +Create `skills/infrahub-managing-generators/rules/python-relationship-references.md`: + +````markdown +--- +title: Passing Relationships to client.create +impact: CRITICAL +tags: python, client, create, hfid, relationship, id +--- + +## Passing Relationships to client.create + +Impact: CRITICAL + +The SDK accepts three explicit forms for relationship fields in +the ``data=`` dict of ``client.create()``. Mixing them up — or +passing a bare string — is the most common source of "Unable to +find the node" errors at runtime. + +### The three accepted forms + +**Form A — HFID lookup (single-component).** Use when the target +schema's ``human_friendly_id`` is one field. The list contains +exactly one string. + +```python +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": {"hfid": ["cEdge-1000"]}, + "platform": {"hfid": ["ios-xe"]}, + }, +) +``` + +**Form B — HFID lookup (composite).** Use when +``human_friendly_id`` has multiple components. The list order +**must** match the schema's declared order. + +```python +# Schema: DcimInterface.human_friendly_id = [device__name__value, name__value] +await self.client.create( + kind="DcimInterface", + data={ + "name": "Ethernet1/1", + "device": {"hfid": ["spine-01"]}, + "connected_to": {"hfid": ["leaf-01", "Ethernet2/2"]}, + }, +) +``` + +**Form C — Explicit ID.** Use when you already hold the UUID +(returned by a prior query). + +```python +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "site": {"id": site_uuid}, + }, +) +``` + +**Form D — SDK object reference (passed directly).** Pass an +object previously returned by ``self.client.get`` or +``self.client.create``. + +```python +site = await self.client.get(kind="LocationSite", name__value="PAR-1") +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "site": site, + }, +) +``` + +### Anti-pattern: bare string + +A bare string for a relationship field is interpreted as +``{"id": ""}``. The server then tries to look up the +string as a UUID, fails, and returns "Unable to find the node". + +```python +# WRONG — server returns "Unable to find the node cEdge-1000 / DcimDeviceType" +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": "cEdge-1000", # treated as id, not HFID + }, +) +``` + +### Anti-pattern: over-packed HFID list + +The HFID list length **must match** the schema's +``human_friendly_id`` declaration. Padding a single-component +HFID with extra fields causes the lookup to fail. + +```python +# WRONG — DcimDeviceType.human_friendly_id is [name__value], single-component. +# Adding manufacturer to the list breaks the lookup. +await self.client.create( + kind="DcimDevice", + data={ + "device_type": {"hfid": ["cEdge-1000", "Cisco"]}, # over-packed + }, +) +``` + +### How to know which form to use + +1. Find the target relationship's ``peer:`` kind in the schema. +2. Look up that node's ``human_friendly_id:`` list. The length + tells you how many strings go in the ``hfid`` list. +3. If you already hold a UUID, use ``{"id": ...}``. Otherwise + prefer ``{"hfid": [...]}``. +4. If you have an SDK object in scope (from ``client.get`` or + ``client.create``), pass it directly — Form D. + +Reference: [Infrahub Generator Docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Write the failing check tests** + +Append to `tests/graders/test_generators_lib.py`: + +```python +# Pull the check functions out of CHECKS as they're added +check_relationship_hfid_form = _mod.CHECKS.get("relationship-hfid-form-correct") +check_no_bare_string_relationship = _mod.CHECKS.get("no-bare-string-relationship") +check_no_overpacked_hfid = _mod.CHECKS.get("no-overpacked-hfid-list") + + +RELATIONSHIP_FIELDS = { + "device_type", "manufacturer", "platform", "site", "location", + "device", "connected_to", +} + + +SRC_GOOD_HFID = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": {"hfid": ["cEdge-1000"]}, + "platform": {"hfid": ["ios-xe"]}, + }, + ) +""" + +SRC_BARE_STRING = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": "cEdge-1000", + }, + ) +""" + +SRC_OVERPACKED_HFID = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "device_type": {"hfid": ["cEdge-1000", "Cisco"]}, + }, + ) +""" + + +def test_relationship_hfid_form_correct_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["relationship-hfid-form-correct"](tree) + assert ok, msg + + +def test_relationship_hfid_form_correct_fails_on_bare_string(): + tree = ast.parse(SRC_BARE_STRING) + ok, msg = _mod.CHECKS["relationship-hfid-form-correct"](tree) + assert not ok + assert "device_type" in msg + + +def test_no_bare_string_relationship_fails_on_bare(): + tree = ast.parse(SRC_BARE_STRING) + ok, msg = _mod.CHECKS["no-bare-string-relationship"](tree) + assert not ok + + +def test_no_bare_string_relationship_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["no-bare-string-relationship"](tree) + assert ok + + +def test_no_overpacked_hfid_fails_on_overpacked(): + tree = ast.parse(SRC_OVERPACKED_HFID) + ok, msg = _mod.CHECKS["no-overpacked-hfid-list"](tree) + assert not ok + assert "device_type" in msg + + +def test_no_overpacked_hfid_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["no-overpacked-hfid-list"](tree) + assert ok +``` + +- [ ] **Step 3: Run tests to verify they fail** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "hfid or bare_string or overpacked" +``` + +Expected: 6 tests fail with `TypeError: 'NoneType' object is not callable` +(the CHECKS entries don't exist yet). + +- [ ] **Step 4: Add check functions to lib.py** + +Append to `graders/managing-generators/lib.py` (before the +`CHECKS = {}` line, then update CHECKS): + +```python +# --------------------------------------------------------------------------- +# Relationship field checks +# --------------------------------------------------------------------------- + +# Relationship-like attribute names commonly used in this repo's base +# schema. The check inspects only these keys in the ``data=`` dict. +# Attribute-style scalar fields (name, status, description, etc.) are +# intentionally excluded so a bare ``"name": "x"`` doesn't fail. +_RELATIONSHIP_KEYS = { + "device_type", "manufacturer", "platform", "site", "location", + "device", "interface", "connected_to", "endpoint_a", "endpoint_z", + "role", "tenant", "provider", "circuit", "asn", "vlan", "vrf", + "ip_namespace", "address", "prefix", "group", +} + + +def _is_known_relationship_key(key: str) -> bool: + return key in _RELATIONSHIP_KEYS + + +def check_relationship_hfid_form_correct( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """For each relationship field in client.create(data=...), the value is + one of: HFID dict, ID dict, or a Name/Attribute reference. + + Bare strings, list literals (non-HFID), or numeric literals fail. + """ + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + problems: list[str] = [] + for call in creates: + data_items = get_data_dict_items(call) + for key, value in data_items.items(): + if not _is_known_relationship_key(key): + continue + hfid_ok, _ = is_hfid_dict(value) + if hfid_ok or is_id_dict(value) or is_name_or_attribute(value): + continue + problems.append(f"{key}: {ast.dump(value)[:60]}") + + if problems: + return False, f"Wrong relationship reference shape: {'; '.join(problems)}" + return True, "All relationship references use HFID dict, ID dict, or SDK object" + + +def check_no_bare_string_relationship( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Relationship fields must not be bare string literals (bug 3 pattern).""" + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + bad: list[str] = [] + for call in creates: + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_bare_string(value): + bad.append(f"{key}={value.value!r}") + + if bad: + return False, f"Bare-string relationship values (treated as id): {', '.join(bad)}" + return True, "No bare-string relationship values found" + + +def check_no_overpacked_hfid_list( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Single-component HFID targets must not receive a multi-string HFID list. + + Heuristic: for the relationships that resolve to single-component HFIDs in + Infrahub's base schema (DcimDeviceType, DcimPlatform, OrganizationManufacturer + referenced via device_type/platform/manufacturer), the HFID list must be + length 1. + """ + SINGLE_COMPONENT_RELATIONSHIPS = { + "device_type", "manufacturer", "platform", "site", "location", + "role", "tenant", "provider", + } + + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + bad: list[str] = [] + for call in creates: + for key, value in get_data_dict_items(call).items(): + if key not in SINGLE_COMPONENT_RELATIONSHIPS: + continue + hfid_ok, elts = is_hfid_dict(value) + if hfid_ok and elts is not None and len(elts) > 1: + bad.append(f"{key} got HFID list of length {len(elts)}") + + if bad: + return False, f"Over-packed HFID list: {', '.join(bad)}" + return True, "No over-packed HFID lists for single-component targets" +``` + +Update the `CHECKS` registry at the bottom of the file: + +```python +CHECKS: dict[str, Any] = { + "relationship-hfid-form-correct": check_relationship_hfid_form_correct, + "no-bare-string-relationship": check_no_bare_string_relationship, + "no-overpacked-hfid-list": check_no_overpacked_hfid_list, +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "hfid or bare_string or overpacked" +``` + +Expected: all 6 tests pass. + +- [ ] **Step 6: Write the task grader script** + +Create `graders/managing-generators/check_relationship_hfid_encoding.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the generator-relationship-hfid-encoding eval. + +Reads ``output.py`` from the CWD and prints skillgrade JSON to stdout. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "relationship-hfid-form-correct", + "no-bare-string-relationship", + "no-overpacked-hfid-list", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) +``` + +- [ ] **Step 7: Verify the grader against hand-crafted fixtures** + +```bash +mkdir -p /tmp/grader-test-hfid/{pass,fail} +cat > /tmp/grader-test-hfid/pass/output.py <<'PY' +async def generate(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": {"hfid": ["cEdge-1000"]}, + "manufacturer": {"hfid": ["Cisco"]}, + }, + ) +PY +cat > /tmp/grader-test-hfid/fail/output.py <<'PY' +async def generate(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": "cEdge-1000", + "manufacturer": {"hfid": ["Cisco", "extra"]}, + }, + ) +PY +cd /tmp/grader-test-hfid/pass && \ + python "$OLDPWD/graders/managing-generators/check_relationship_hfid_encoding.py" +echo "---" +cd /tmp/grader-test-hfid/fail && \ + python "$OLDPWD/graders/managing-generators/check_relationship_hfid_encoding.py" +cd "$OLDPWD" +``` + +Expected output: + +```text +{"score": 1.0, "details": "All 3 checks passed.", "checks": [...]} +--- +{"score": 0.0, "details": "0/3 checks passed. Failed: ...", "checks": [...]} +``` + +If the fail fixture scores > 0.0, the check is broken — fix it +before continuing. + +- [ ] **Step 8: Add the eval task to eval.yaml** + +Append to the `tasks:` list in `eval.yaml`: + +```yaml + # --- Generators: relationship reference encoding --- # + - name: generator-relationship-hfid-encoding + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a DcimDevice named 'cEdge-1' + with these single-component HFID references on the schema: + - device_type: 'cEdge-1000' (DcimDeviceType.human_friendly_id = [name__value]) + - manufacturer: 'Cisco' (OrganizationManufacturer.human_friendly_id = [name__value]) + - platform: 'ios-xe' (DcimPlatform.human_friendly_id = [name__value]) + + The generator must use the documented HFID dict form for each relationship, + not bare strings, and the HFID lists must be single-element (matching the + schema's human_friendly_id length). + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_relationship_hfid_encoding.py + weight: 1.0 + expected_output: >- + A Python generator class that calls self.client.create with each + relationship field set to {"hfid": [""]} — a single-element + list inside a dict with the literal key "hfid". No bare strings. + expectations: + - >- + Generator inherits from InfrahubGenerator and implements async generate() + - >- + Each relationship field uses the {"hfid": [""]} dict form + - >- + No relationship field is a bare string (would be treated as id) + - >- + HFID lists for single-component targets are length 1 (no over-pack) + - "save() is called with allow_upsert=True" + assertions: + - name: relationship-hfid-form-correct + check: >- + Every relationship key in client.create(data=...) is either a + {"hfid": [...]} dict, {"id": "..."} dict, or a variable reference. + - name: no-bare-string-relationship + check: >- + No relationship key has a bare string literal as its value. + - name: no-overpacked-hfid-list + check: >- + Single-component HFID targets (device_type, manufacturer, platform) + receive an HFID list of length 1. +``` + +- [ ] **Step 9: Commit** + +```bash +git add \ + skills/infrahub-managing-generators/rules/python-relationship-references.md \ + graders/managing-generators/lib.py \ + graders/managing-generators/check_relationship_hfid_encoding.py \ + tests/graders/test_generators_lib.py \ + eval.yaml +git commit -m "feat(generators): rule + grader for HFID relationship encoding" +``` + +--- + +## Task 4: Three-forms variant (rule already covers it, just add grader + eval) + +**Files:** + +- Modify: `graders/managing-generators/lib.py` (add 3 checks) +- Modify: `tests/graders/test_generators_lib.py` (add tests) +- Create: `graders/managing-generators/check_relationship_three_forms.py` +- Modify: `eval.yaml` + +This task exercises all three forms (HFID dict, ID dict, SDK +object reference) in one generator. The rule +`python-relationship-references.md` already documents the three +forms — this task adds graders + an eval task that confirms the +model uses each form when appropriate. + +- [ ] **Step 1: Write check tests** + +Append to `tests/graders/test_generators_lib.py`: + +```python +SRC_THREE_FORMS = """ +async def generate(self): + site = await self.client.get(kind="LocationSite", name__value="PAR-1") + device_type_uuid = "11111111-1111-1111-1111-111111111111" + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "manufacturer": {"hfid": ["Cisco"]}, + "device_type": {"id": device_type_uuid}, + "site": site, + }, + ) +""" + + +def test_hfid_form_for_name_lookup_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["hfid-form-for-name-lookup"](tree) + assert ok, msg + + +def test_id_form_for_uuid_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["id-form-for-uuid"](tree) + assert ok, msg + + +def test_sdk_object_reference_used_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["sdk-object-reference-used"](tree) + assert ok, msg + + +def test_three_forms_fail_when_all_hfid(): + src = """ +async def generate(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "x", + "manufacturer": {"hfid": ["Cisco"]}, + "device_type": {"hfid": ["cEdge-1000"]}, + "site": {"hfid": ["PAR-1"]}, + }, + ) +""" + tree = ast.parse(src) + ok, _ = _mod.CHECKS["id-form-for-uuid"](tree) + assert not ok + ok, _ = _mod.CHECKS["sdk-object-reference-used"](tree) + assert not ok +``` + +- [ ] **Step 2: Run tests to confirm they fail** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "three_forms or hfid_form or id_form or sdk_object" +``` + +Expected: 4 tests fail with `KeyError` (CHECKS entries don't exist yet). + +- [ ] **Step 3: Add check functions to lib.py** + +Append to `graders/managing-generators/lib.py` after the +existing relationship checks: + +```python +def check_hfid_form_for_name_lookup( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create uses {"hfid": [...]}.""" + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key): + ok, _ = is_hfid_dict(value) + if ok: + return True, f"{key} uses HFID dict form" + return False, "No relationship uses {'hfid': [...]} form" + + +def check_id_form_for_uuid( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create uses {"id": ...}.""" + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_id_dict(value): + return True, f"{key} uses ID dict form" + return False, "No relationship uses {'id': ...} form" + + +def check_sdk_object_reference_used( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create is a variable reference + (presumably an SDK object from a prior client.get / client.create call). + """ + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_name_or_attribute(value): + return True, f"{key} uses SDK object reference" + return False, "No relationship passes an SDK object reference" +``` + +Add to `CHECKS`: + +```python +CHECKS["hfid-form-for-name-lookup"] = check_hfid_form_for_name_lookup +CHECKS["id-form-for-uuid"] = check_id_form_for_uuid +CHECKS["sdk-object-reference-used"] = check_sdk_object_reference_used +``` + +- [ ] **Step 4: Run tests to confirm they pass** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "three_forms or hfid_form or id_form or sdk_object" +``` + +Expected: 4 tests pass. + +- [ ] **Step 5: Write the task grader** + +Create `graders/managing-generators/check_relationship_three_forms.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the generator-relationship-three-forms eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "hfid-form-for-name-lookup", + "id-form-for-uuid", + "sdk-object-reference-used", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) +``` + +- [ ] **Step 6: Add eval task** + +Append to `eval.yaml`: + +```yaml + - name: generator-relationship-three-forms + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a DcimDevice 'cEdge-1' and + sets its relationships using all three forms documented in the skill: + + - manufacturer: by HFID (the name 'Cisco' — DcimDevice.manufacturer is a + single-component HFID). + - device_type: by explicit ID, where the UUID is obtained from a prior + GraphQL query and is available in scope as a string variable. + - site: by SDK object reference, where you first call self.client.get + for the LocationSite and pass the returned object directly. + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_relationship_three_forms.py + weight: 1.0 + expected_output: >- + A Python generator that exercises each of the three relationship + reference forms (HFID dict, ID dict, SDK object reference) on the + same client.create call. + expectations: + - "At least one relationship uses {'hfid': ['']}" + - "At least one relationship uses {'id': }" + - >- + At least one relationship passes a variable (SDK object) from + a prior self.client.get call + assertions: + - name: hfid-form-for-name-lookup + check: At least one relationship uses {'hfid': [...]} form + - name: id-form-for-uuid + check: At least one relationship uses {'id': ...} form + - name: sdk-object-reference-used + check: At least one relationship is a variable reference +``` + +- [ ] **Step 7: Verify the grader works** + +```bash +mkdir -p /tmp/grader-test-three-forms/pass +cat > /tmp/grader-test-three-forms/pass/output.py <<'PY' +async def generate(self): + site = await self.client.get(kind="LocationSite", name__value="PAR-1") + uuid_var = "11111111-1111-1111-1111-111111111111" + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "manufacturer": {"hfid": ["Cisco"]}, + "device_type": {"id": uuid_var}, + "site": site, + }, + ) +PY +cd /tmp/grader-test-three-forms/pass && \ + python "$OLDPWD/graders/managing-generators/check_relationship_three_forms.py" +cd "$OLDPWD" +``` + +Expected: `{"score": 1.0, ...}`. + +- [ ] **Step 8: Commit** + +```bash +git add \ + graders/managing-generators/lib.py \ + graders/managing-generators/check_relationship_three_forms.py \ + tests/graders/test_generators_lib.py \ + eval.yaml +git commit -m "feat(generators): grader + eval for three-form relationship references" +``` + +--- + +## Task 5: Bug 4 — Multi-peer add (rule + grader + eval) + +**Files:** + +- Create: `skills/infrahub-managing-generators/rules/python-multi-peer-add.md` +- Modify: `graders/managing-generators/lib.py` +- Modify: `tests/graders/test_generators_lib.py` +- Create: `graders/managing-generators/check_multi_peer_iteration.py` +- Modify: `eval.yaml` +- [ ] **Step 1: Write the rule file** + +Create `skills/infrahub-managing-generators/rules/python-multi-peer-add.md`: + +````markdown +--- +title: Adding Multiple Peers to a Relationship +impact: HIGH +tags: python, relationship, add, members, manager, peers +--- + +## Adding Multiple Peers to a Relationship + +Impact: HIGH + +``RelationshipManager.add()`` takes **one peer per call**. Passing a +list of peers creates a single peer whose HFID is the list contents, +which the server rejects (or worse, accepts as malformed data). + +### How the bug looks + +The mutation generated by passing a list to ``.add()`` looks like: + +```text +members: [{hfid: [, , ]}] +``` + +— a single peer with a composite HFID equal to the entire list. The +server returns "Unable to find the node" or "Invalid HFID +component count" depending on the schema. + +### Anti-pattern + +```python +# WRONG — interprets the whole list as one peer +edges_to_add: list[CoreNode] = [d1, d2, d3] +group.members.add(edges_to_add) +await group.save() +``` + +### Correct pattern + +```python +# RIGHT — one peer per .add() call +edges_to_add: list[CoreNode] = [d1, d2, d3] +for peer in edges_to_add: + group.members.add(peer) +await group.save() +``` + +The same constraint applies to other ``RelationshipManager`` +mutators (``.update``, ``.remove``). + +### Why iterate, not collect into a list comprehension that's then + +passed to ``.add()``? + +``RelationshipManager.add(peer)`` mutates internal state in place; +the return value is not the new list. So +``[mgr.add(p) for p in peers]`` is equivalent to the correct pattern +above, but is harder to read and adds no value. Prefer the explicit +``for`` loop. + +Reference: [Infrahub SDK Docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Write the failing check tests** + +Append to `tests/graders/test_generators_lib.py`: + +```python +SRC_GOOD_ADD_LOOP = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + devices = [d1, d2, d3] + for peer in devices: + group.members.add(peer) + await group.save() +""" + +SRC_BAD_ADD_LIST = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + devices = [d1, d2, d3] + group.members.add(devices) + await group.save() +""" + +SRC_BAD_ADD_LIST_LITERAL = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + group.members.add([d1, d2, d3]) + await group.save() +""" + + +def test_no_list_passed_to_add_passes_on_iteration(): + tree = ast.parse(SRC_GOOD_ADD_LOOP) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert ok, msg + + +def test_no_list_passed_to_add_fails_on_list_literal(): + tree = ast.parse(SRC_BAD_ADD_LIST_LITERAL) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert not ok + + +def test_no_list_passed_to_add_fails_on_list_named_variable(): + tree = ast.parse(SRC_BAD_ADD_LIST) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert not ok + + +def test_members_add_iterates_passes_on_loop(): + tree = ast.parse(SRC_GOOD_ADD_LOOP) + ok, msg = _mod.CHECKS["members-add-iterates"](tree) + assert ok, msg + + +def test_members_add_iterates_fails_on_no_loop(): + tree = ast.parse(SRC_BAD_ADD_LIST_LITERAL) + ok, msg = _mod.CHECKS["members-add-iterates"](tree) + assert not ok +``` + +- [ ] **Step 3: Run tests to confirm they fail** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "add_list or add_iterate or members_add" +``` + +Expected: 5 tests fail with `KeyError`. + +- [ ] **Step 4: Add check functions to lib.py** + +Append to `graders/managing-generators/lib.py`: + +```python +# --------------------------------------------------------------------------- +# Multi-peer add checks +# --------------------------------------------------------------------------- + + +def _is_list_referenced(node: ast.AST, tree: ast.Module) -> bool: + """Best-effort: is ``node`` either a list literal or a Name that was + assigned a list literal earlier in the module? + """ + if isinstance(node, ast.List): + return True + if isinstance(node, ast.Name): + target_name = node.id + for assign in ast.walk(tree): + if not isinstance(assign, ast.Assign): + continue + for tgt in assign.targets: + if isinstance(tgt, ast.Name) and tgt.id == target_name: + if isinstance(assign.value, ast.List): + return True + if isinstance(assign.value, ast.ListComp): + return True + # Also check ann-assigns: `devices: list[T] = [...]` + for ann in ast.walk(tree): + if not isinstance(ann, ast.AnnAssign): + continue + tgt = ann.target + if isinstance(tgt, ast.Name) and tgt.id == target_name: + if isinstance(ann.value, (ast.List, ast.ListComp)): + return True + return False + + +def check_no_list_passed_to_add( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """No .add(...) call may receive a list argument as its sole peer.""" + if tree is None: + return False, "No Python source to inspect" + + add_calls = find_relationship_add_calls(tree) + if not add_calls: + return False, "No .add(...) calls found" + + bad: list[str] = [] + for call in add_calls: + if len(call.args) != 1: + continue + arg = call.args[0] + if _is_list_referenced(arg, tree): + bad.append(ast.unparse(call.func) if hasattr(ast, "unparse") else call.func.attr) + + if bad: + return False, f".add() received a list: {', '.join(bad)}" + return True, "No .add() call received a list argument" + + +def check_members_add_iterates( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """``.add(...)`` should appear inside a For loop OR be called multiple times + on the same attribute path. Both indicate per-peer iteration. + """ + if tree is None: + return False, "No Python source to inspect" + + add_calls = find_relationship_add_calls(tree) + if not add_calls: + return False, "No .add(...) calls found" + + # Look for any For loop containing a .add() call + for for_node in ast.walk(tree): + if not isinstance(for_node, ast.For): + continue + for inner in ast.walk(for_node): + if isinstance(inner, ast.Call) and inner in add_calls: + return True, ".add() is called inside a for loop" + + # If multiple .add() calls share the same attribute path, treat as + # explicit per-peer adds + paths = [] + for call in add_calls: + try: + paths.append(ast.unparse(call.func)) + except Exception: + pass + if len(paths) >= 2 and len(set(paths)) <= len(paths): + return True, f".add() called multiple times: {len(add_calls)} calls" + + return False, "Only a single .add() call and not in a for loop" +``` + +Add to `CHECKS`: + +```python +CHECKS["no-list-passed-to-add"] = check_no_list_passed_to_add +CHECKS["members-add-iterates"] = check_members_add_iterates +``` + +- [ ] **Step 5: Run tests to confirm they pass** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "add_list or add_iterate or members_add" +``` + +Expected: 5 tests pass. + +- [ ] **Step 6: Write the task grader** + +Create `graders/managing-generators/check_multi_peer_iteration.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the generator-multi-peer-iteration eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "no-list-passed-to-add", + "members-add-iterates", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) +``` + +- [ ] **Step 7: Add eval task** + +Append to `eval.yaml`: + +```yaml + - name: generator-multi-peer-iteration + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a CoreStandardGroup named + 'sdwan-edges' and adds five DcimDevice instances ('edge-1' through + 'edge-5') as members of that group via group.members.add(). + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_multi_peer_iteration.py + weight: 1.0 + expected_output: >- + A Python generator that adds each peer to the group with one .add() + call per peer (typically inside a for loop), then a single .save(). + No list is passed to .add() in a single call. + expectations: + - "Group is created or fetched via self.client.get / self.client.create" + - ".add() is called once per peer (inside a for loop or 5 explicit calls)" + - "No call passes a list of peers as a single .add() argument" + assertions: + - name: no-list-passed-to-add + check: No .add() call receives a list as its argument + - name: members-add-iterates + check: .add() is invoked inside a for loop or multiple times explicitly +``` + +- [ ] **Step 8: Commit** + +```bash +git add \ + skills/infrahub-managing-generators/rules/python-multi-peer-add.md \ + graders/managing-generators/lib.py \ + graders/managing-generators/check_multi_peer_iteration.py \ + tests/graders/test_generators_lib.py \ + eval.yaml +git commit -m "feat(generators): rule + grader for multi-peer .add() iteration" +``` + +--- + +## Task 6: Bug 5 — Natural-key preflight (rule + grader + eval) + +**Files:** + +- Create: `skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md` +- Modify: `graders/managing-generators/lib.py` +- Modify: `tests/graders/test_generators_lib.py` +- Create: `graders/managing-generators/check_natural_key_preflight.py` +- Modify: `eval.yaml` +- [ ] **Step 1: Write the rule** + +Create `skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md`: + +````markdown +--- +title: Pre-flight Natural-Key Check for Form-Driven Mutations +impact: MEDIUM +tags: patterns, idempotent, uniqueness, upsert, form, catalog +--- + +## Pre-flight Natural-Key Check for Form-Driven Mutations + +Impact: MEDIUM + +Inside ``InfrahubGenerator.generate()`` the default ``save()`` +already happens with ``allow_upsert=True`` because the tracking +context wraps the call. **Outside that context** — in scripts, +Streamlit pages, or any ad-hoc code that takes user input and +calls ``client.create`` directly — uniqueness collisions on +bootstrap-seeded keys surface as raw server exceptions in the +middle of a half-built workflow. + +Always pick one of two patterns for catalog-style mutations. + +### Pattern A — pre-flight check + +Use when the desired behavior is "fail loudly with a friendly +message if the object already exists." + +```python +from infrahub_sdk.exceptions import NodeNotFound + +try: + existing = await client.get( + kind="IpamPrefix", + prefix__value=user_input_prefix, + ) + st.error(f"Prefix {user_input_prefix} already exists.") + return +except NodeNotFound: + pass + +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save() +``` + +### Pattern B — upsert + +Use when the desired behavior is "create or update silently." +This is the default inside generators. + +```python +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save(allow_upsert=True) +``` + +### Anti-pattern + +```python +# WRONG — uniqueness collision surfaces as raw server exception +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save() # no upsert, no preflight +``` + +### How to decide + +- **Form-driven UI catalog pages:** Pattern A. Give the user a + legible "already exists" message. +- **Generators with tracking:** ``allow_upsert=True`` is the + norm — the generator should be idempotent. +- **Bootstrap scripts and migrations:** Pattern B (upsert) + unless you have a specific reason to fail on existing data. + +Reference: [Infrahub SDK Docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Write failing tests** + +Append to `tests/graders/test_generators_lib.py`: + +```python +SRC_PREFLIGHT = """ +from infrahub_sdk.exceptions import NodeNotFound + +async def create_prefix(client, user_prefix): + try: + existing = await client.get(kind="IpamPrefix", prefix__value=user_prefix) + return existing + except NodeNotFound: + pass + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save() +""" + +SRC_UPSERT = """ +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save(allow_upsert=True) +""" + +SRC_UNSAFE = """ +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save() +""" + + +def test_preflight_or_upsert_passes_on_preflight(): + tree = ast.parse(SRC_PREFLIGHT) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert ok + + +def test_preflight_or_upsert_passes_on_upsert(): + tree = ast.parse(SRC_UPSERT) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert ok + + +def test_preflight_or_upsert_fails_on_unsafe(): + tree = ast.parse(SRC_UNSAFE) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert not ok +``` + +- [ ] **Step 3: Run tests to confirm they fail** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "preflight_or_upsert" +``` + +Expected: 3 tests fail with `KeyError`. + +- [ ] **Step 4: Add check function to lib.py** + +The check inspects the tree for either ``client.get`` preceding +``client.create`` for the same kind, or any ``.save(allow_upsert=True)`` +call. (A loose heuristic — the rule's spirit is "don't go raw".) + +Append to `graders/managing-generators/lib.py`: + +```python +# --------------------------------------------------------------------------- +# Natural-key preflight +# --------------------------------------------------------------------------- + + +def _has_save_with_upsert_true(tree: ast.Module) -> bool: + """True if any ``.save(allow_upsert=True)`` call exists.""" + for call in _iter_calls(tree): + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != "save": + continue + for kw in call.keywords: + if kw.arg == "allow_upsert": + if isinstance(kw.value, ast.Constant) and kw.value.value is True: + return True + return False + + +def _has_client_get_for_same_kind_as_create(tree: ast.Module) -> bool: + """True if there is at least one ``client.get(kind=X)`` AND a + ``client.create(kind=X)`` for the same kind value. + """ + def _kind_arg(call: ast.Call) -> str | None: + for kw in call.keywords: + if kw.arg == "kind" and isinstance(kw.value, ast.Constant): + return kw.value.value + return None + + creates = find_client_create_calls(tree) + gets = find_client_get_calls(tree) + + # Also accept generic ``client.get`` (not self.client.get) for non-generator scripts + for call in _iter_calls(tree): + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != "get": + continue + if isinstance(func.value, ast.Name) and func.value.id == "client": + gets.append(call) + + create_kinds = {_kind_arg(c) for c in creates if _kind_arg(c)} + get_kinds = {_kind_arg(c) for c in gets if _kind_arg(c)} + return bool(create_kinds & get_kinds) + + +def check_preflight_or_upsert( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Either a same-kind client.get precedes create, OR save uses upsert.""" + if tree is None: + return False, "No Python source to inspect" + + if _has_save_with_upsert_true(tree): + return True, "save(allow_upsert=True) found" + if _has_client_get_for_same_kind_as_create(tree): + return True, "client.get preflights client.create for the same kind" + return False, "Neither preflight client.get nor save(allow_upsert=True) found" +``` + +Register: + +```python +CHECKS["preflight-or-upsert"] = check_preflight_or_upsert +``` + +Add a second check for the no-raw-create case: + +```python +def check_no_raw_create_without_handler( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """If client.create is used but neither preflight nor upsert is present, + this is the bug 5 pattern. + """ + if tree is None: + return False, "No Python source to inspect" + + if not find_client_create_calls(tree) and \ + not any( + isinstance(c.func, ast.Attribute) and c.func.attr == "create" + and isinstance(c.func.value, ast.Name) and c.func.value.id == "client" + for c in _iter_calls(tree) + ): + return False, "No client.create call found" + + if _has_save_with_upsert_true(tree): + return True, "save(allow_upsert=True) covers the collision case" + if _has_client_get_for_same_kind_as_create(tree): + return True, "preflight client.get covers the collision case" + return False, "client.create has no preflight and no allow_upsert=True" + + +CHECKS["no-raw-create-without-handler"] = check_no_raw_create_without_handler +``` + +- [ ] **Step 5: Run tests to confirm they pass** + +```bash +pytest tests/graders/test_generators_lib.py -v -k "preflight_or_upsert" +``` + +Expected: 3 tests pass. + +- [ ] **Step 6: Write the task grader** + +Create `graders/managing-generators/check_natural_key_preflight.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the generator-natural-key-preflight eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "preflight-or-upsert", + "no-raw-create-without-handler", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) +``` + +- [ ] **Step 7: Add eval task** + +Append to `eval.yaml`: + +```yaml + - name: generator-natural-key-preflight + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write a Python script that creates an IpamPrefix '10.250.10.0/24' + from user input. The prefix may already have been seeded by an earlier + bootstrap run. Handle the collision gracefully — either pre-flight check + for an existing prefix (and exit with a clear message) or save with + allow_upsert=True. Do not let a raw server-side uniqueness error escape. + + Save ONLY the final Python script to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_natural_key_preflight.py + weight: 1.0 + expected_output: >- + A Python script that either uses client.get with the natural key + before calling client.create, or passes allow_upsert=True to save(). + expectations: + - "Calls client.create(kind='IpamPrefix', ...)" + - >- + Either calls client.get(kind='IpamPrefix', ...) before create, + or passes allow_upsert=True to save() + - "No bare client.create + save() without one of the two patterns" + assertions: + - name: preflight-or-upsert + check: >- + Either client.get(kind='IpamPrefix', ...) precedes create, OR + save(allow_upsert=True) is called. + - name: no-raw-create-without-handler + check: >- + client.create has either a preflight get OR allow_upsert=True. +``` + +- [ ] **Step 8: Verify the grader** + +```bash +mkdir -p /tmp/grader-test-preflight/{pass,fail} +cat > /tmp/grader-test-preflight/pass/output.py <<'PY' +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save(allow_upsert=True) +PY +cat > /tmp/grader-test-preflight/fail/output.py <<'PY' +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save() +PY +cd /tmp/grader-test-preflight/pass && \ + python "$OLDPWD/graders/managing-generators/check_natural_key_preflight.py" +echo "---" +cd /tmp/grader-test-preflight/fail && \ + python "$OLDPWD/graders/managing-generators/check_natural_key_preflight.py" +cd "$OLDPWD" +``` + +Expected: pass scores 1.0, fail scores 0.0. + +- [ ] **Step 9: Commit** + +```bash +git add \ + skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md \ + graders/managing-generators/lib.py \ + graders/managing-generators/check_natural_key_preflight.py \ + tests/graders/test_generators_lib.py \ + eval.yaml +git commit -m "feat(generators): rule + grader for natural-key preflight" +``` + +--- + +## Task 7: Integration-testing rule + advisory eval task + +**Files:** + +- Create: `skills/infrahub-managing-generators/rules/testing-integration.md` +- Modify: `eval.yaml` + +The rule is advisory (process guidance with no observable +structural outcome in the generator source). The eval task uses +``expectations`` only, no deterministic grader script. + +- [ ] **Step 1: Write the rule** + +Create `skills/infrahub-managing-generators/rules/testing-integration.md`: + +````markdown +--- +title: Run Generators End-to-End Before Declaring Done +impact: LOW +tags: testing, integration, infrahubctl, runtime, verification +--- + +## Run Generators End-to-End Before Declaring Done + +Impact: LOW + +Unit tests on the input dict do not cover SDK call shape. +Bug classes that pass unit tests but fail at runtime against a +real Infrahub server include: + +- HFID encoded as a bare string (treated as ``id``) +- Over-packed HFID list for a single-component target +- List passed to ``RelationshipManager.add`` instead of iterating +- Uniqueness collisions on bootstrap-seeded keys + +Every one of these has the same property: the Python code is +syntactically and type-wise fine; only the wire protocol shape is +wrong. **Run the generator against a live test instance before +declaring it done.** + +### Concrete workflow + +After the generator is implemented and unit-tested: + +1. ``infrahubctl generator list`` — confirm the new definition + registered. +2. ``infrahubctl generator run `` — execute + the generator against a real branch. +3. Verify the created objects exist via the UI or a GraphQL + query. Confirm relationships resolve. +4. If anything fails, fix and re-run before moving on. + +### When this matters most + +- Pre-PR self-review on a development branch. +- After any change to relationship reference shape. +- After any schema migration that changes ``human_friendly_id``. + +Unit tests are still valuable for ``clean_data()`` helpers, branch +logic, and pure-Python transforms — they just don't replace the +end-to-end run. + +Reference: [Infrahub Generator Docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Add advisory eval task** + +Append to `eval.yaml`: + +```yaml + - name: generator-process-integration-test + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates three DcimDevice instances + from a TopologyDataCenter design. After the generator class, include a + short section (as a Python triple-quoted string named TEST_PLAN at the + bottom of the file, or as a # block comment) explaining how to test the + generator end-to-end against a live Infrahub instance before merging. + + Save ONLY the final Python file to: output.py + graders: [] + expected_output: >- + A Python generator plus end-to-end test guidance. The guidance should + mention infrahubctl generator run against a live instance and note + that input-dict unit tests don't cover SDK call shape. + expectations: + - "Generator class is present and creates DcimDevice instances" + - "TEST_PLAN string or block comment exists at the bottom of the file" + - >- + The guidance mentions infrahubctl generator run against a real + Infrahub instance + - >- + The guidance notes that unit tests on the input dict do not cover + SDK call shape +``` + +The empty ``graders: []`` is intentional — skillgrade treats this +as a manual-review task. No grader script is created for it. + +- [ ] **Step 3: Commit** + +```bash +git add \ + skills/infrahub-managing-generators/rules/testing-integration.md \ + eval.yaml +git commit -m "feat(generators): advisory rule + eval task for integration testing" +``` + +--- + +## Task 8: Fix the misleading example in python-generate.md + +**Files:** + +- Modify: `skills/infrahub-managing-generators/rules/python-generate.md` + +The current example uses ``device_type_id`` as a singular variable +without shape hints, and the comment ``# Use ID for relationships`` +is what led the user to bare-string a relationship value during +the demo. We update the example to use the documented HFID dict +form and cross-link to ``python-relationship-references.md``. + +- [ ] **Step 1: Read the current file** + +```bash +cat skills/infrahub-managing-generators/rules/python-generate.md +``` + +Confirm the section starting with ``### Object Creation API`` is +present and contains the lines: + +```text + # Use ID for relationships + "device_type": device_type_id, +``` + +- [ ] **Step 2: Replace the misleading example** + +Find the ``### Object Creation API`` block in +`skills/infrahub-managing-generators/rules/python-generate.md` and +replace its content with: + +```markdown +### Object Creation API + +```python +# Create a new object — see [python-relationship-references.md] +# for the three accepted forms for relationship fields. +obj = await self.client.create( + kind="DcimDevice", + data={ + "name": "my-device", + "status": "active", + # HFID dict for single-component HFID targets + "device_type": {"hfid": ["cEdge-1000"]}, + } +) +await obj.save(allow_upsert=True) + +# Get an existing object +existing = await self.client.get( + kind="LocationBuilding", + name__value="PAR-1", +) + +# Allocate from a pool +ip = await self.client.allocate_next_ip_address( + resource_pool=pool, + identifier=f"{device_name}-loopback", +) +``` + +Find the ``### Critical Rules`` block and update the bullet that +reads "Use IDs for relationship references in `data` dict" to: + +```markdown +- Reference related objects in `data` by HFID dict + (`{"hfid": ["name"]}`), ID dict (`{"id": ""}`), or SDK + object directly. A bare string is treated as ``id``, not HFID. + See [python-relationship-references.md](./python-relationship-references.md). +``` + +- [ ] **Step 3: Verify the diff** + +```bash +git diff skills/infrahub-managing-generators/rules/python-generate.md +``` + +Expected: the ``# Use ID for relationships`` line is gone, the +example shows ``{"hfid": ["cEdge-1000"]}``, and the Critical Rules +bullet points to the new rule file. + +- [ ] **Step 4: Commit** + +```bash +git add skills/infrahub-managing-generators/rules/python-generate.md +git commit -m "fix(generators): replace misleading device_type_id example" +``` + +--- + +## Task 9: Update _sections.md + +**Files:** + +- Modify: `skills/infrahub-managing-generators/rules/_sections.md` + +The sections index documents the prefix categories. We extend the +``python-`` and ``patterns-`` and ``testing-`` entries to mention +the new content. + +- [ ] **Step 1: Open `_sections.md` and find the existing entries** + +```bash +cat skills/infrahub-managing-generators/rules/_sections.md +``` + +- [ ] **Step 2: Replace the entries for python-, patterns-, testing-** + +Update the section index so the entries read: + +```markdown +2. **Python Class (python-)** -- CRITICAL. + InfrahubGenerator base class, async generate() method, + object creation via self.client.create(), save with + allow_upsert=True, relationship references via HFID dict / + ID dict / SDK object (never bare string), and multi-peer + add iteration on RelationshipManager. + +6. **Patterns (patterns-)** -- MEDIUM. Data cleaning helper, + batch object creation, using the local store for + inter-object references, and natural-key preflight for + form-driven mutations. + +7. **Testing (testing-)** -- LOW. infrahubctl generator + commands, listing and running Generators locally, and the + integration-test workflow (run end-to-end against a live + instance before declaring done). +``` + +- [ ] **Step 3: Commit** + +```bash +git add skills/infrahub-managing-generators/rules/_sections.md +git commit -m "docs(generators): update _sections.md to mention new rules" +``` + +--- + +## Task 10: Regenerate evaluations/managing-generators.json + +**Files:** + +- Generated: `evaluations/managing-generators.json` + +`sync-evals.py` converts the root `eval.yaml` into per-skill JSON +projections used by the ``/skill-creator`` evals runner. The +script is idempotent and overwrites the JSON file each run. + +- [ ] **Step 1: Run sync-evals** + +```bash +python scripts/sync-evals.py +``` + +Expected output: prints a line per skill being synced, including +``managing-generators`` with 5 tasks. + +- [ ] **Step 2: Inspect the generated file** + +```bash +ls evaluations/managing-generators.json +head -40 evaluations/managing-generators.json +``` + +Expected: the file exists and contains five task entries with +names matching the five we added. + +- [ ] **Step 3: Commit the generated file** + +```bash +git add evaluations/managing-generators.json +git commit -m "chore(evals): regenerate managing-generators JSON projection" +``` + +--- + +## Task 11: Run skillgrade smoke and iterate if needed + +**Files:** (none directly — may modify rule files based on results) + +A smoke run executes each task multiple times (5 trials default in +``--smoke`` mode) and reports the pass rate per assertion. If a +rule's pass rate is below 0.8, the rule prose likely needs +strengthening (clearer WARNING blocks, more concrete examples). + +- [ ] **Step 1: Run smoke against the new tasks** + +```bash +skillgrade --smoke +``` + +Expected: 5 trials × 5 new tasks = 25 runs. Each run produces a +score. The five new tasks should appear in the output. + +- [ ] **Step 2: Inspect failure modes** + +```bash +skillgrade preview +``` + +For any task with pass rate < 0.8: + +- Look at the failed assertions. Does the rule prose actually + forbid the failure mode? +- Add a concrete WRONG/RIGHT example to the rule if missing. +- Strengthen the cross-references between rules. +- [ ] **Step 3: (Conditional) Iterate** + +If iteration is needed, edit the relevant rule file(s) in +`skills/infrahub-managing-generators/rules/` and re-run +`skillgrade --smoke` until pass rates are ≥ 0.8 or the gap is +documented as a known limitation in CHANGELOG. + +- [ ] **Step 4: Commit any rule strengthening** + +```bash +git add skills/infrahub-managing-generators/rules/ +git commit -m "docs(generators): strengthen rules based on smoke results" +``` + +(If no changes were needed, skip the commit.) + +--- + +## Task 12: Update CHANGELOG.md + +**Files:** + +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: Read current CHANGELOG** + +```bash +head -40 CHANGELOG.md +``` + +Identify where the next-version section lives (usually a +``## Unreleased`` block or a placeholder). + +- [ ] **Step 2: Add the new entries** + +Under the appropriate section, add: + +```markdown +### managing-generators + +- New rule `python-relationship-references` covering the three + accepted forms for relationship fields in + `self.client.create()` (HFID dict, ID dict, SDK object) plus + the bare-string and over-packed-list anti-patterns. +- New rule `python-multi-peer-add` explaining that + `RelationshipManager.add()` takes one peer per call; iterate + the peer list. +- New rule `patterns-natural-key-preflight` covering form-driven + mutations that collide with bootstrap-seeded keys. +- New advisory rule `testing-integration` recommending + end-to-end runs against a live instance before merge. +- Fixed misleading `device_type_id` example in `python-generate` + that suggested bare-string relationship values. +- Added 5 deterministic evals + 4 graders to `eval.yaml` + (first evals for this skill). +``` + +- [ ] **Step 3: Commit** + +```bash +git add CHANGELOG.md +git commit -m "docs: changelog entries for managing-generators improvements" +``` + +--- + +## Task 13: Self-review + open draft PR + +**Files:** (none — review and PR action) + +- [ ] **Step 1: Review the diff against `main`** + +```bash +git diff main --stat +git log main..HEAD --oneline +``` + +Confirm: + +- All 4 new rule files exist +- `python-generate.md` is edited +- `_sections.md` is edited +- `graders/managing-generators/` has 4 task graders + lib + init +- `tests/graders/test_generators_lib.py` exists +- `eval.yaml` has 5 new tasks +- `evaluations/managing-generators.json` is regenerated +- `CHANGELOG.md` is updated +- [ ] **Step 2: Run the full pytest suite** + +```bash +pytest tests/ -v +``` + +Expected: existing tests still pass, all new generator lib tests +pass. + +- [ ] **Step 3: Verify markdown lint cleanliness** + +```bash +uvx rumdl skills/infrahub-managing-generators/ +``` + +Expected: no lint errors. If errors appear, fix them and commit. + +- [ ] **Step 4: Push and open draft PR** + +```bash +git push -u origin general-improvements +gh pr create --draft \ + --title "feat(generators): integration-layer rules for HFID encoding, multi-peer adds, preflight" \ + --body "$(cat <<'BODY' +## Summary + +Closes integration-layer gaps in `infrahub-managing-generators` that allowed +four real-world demo bugs (HFID over-pack, bare-string relationship, +multi-peer list, uniqueness collision) plus one cross-cutting process +recommendation. + +Spec: `dev/specs/2026-05-18-managing-generators-integration-rules-design.md` +Plan: `dev/plans/2026-05-18-managing-generators-integration-rules.md` + +### Changes + +- 4 new rules: `python-relationship-references`, + `python-multi-peer-add`, `patterns-natural-key-preflight`, + `testing-integration`. +- Fix misleading example in `python-generate`. +- Update `_sections.md`. +- New `graders/managing-generators/` (lib + 4 task graders). +- 5 new tasks in `eval.yaml` (first evals for this skill). +- Regenerated `evaluations/managing-generators.json`. + +### Test plan + +- [ ] pytest tests/graders/test_generators_lib.py +- [ ] skillgrade --smoke produces ≥ 0.8 pass rate on new tasks +- [ ] manual verification of grader on hand-crafted fixtures + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +BODY +)" +``` + +Per the user's global instruction, the PR is opened as draft. + +--- + +## Self-Review + +Run before declaring the plan complete (already done — captured +in this section). + +**1. Spec coverage:** + +| Spec item | Task | +| --- | --- | +| Edit `python-generate.md` (fix misleading example) | Task 8 | +| New `python-relationship-references.md` | Task 3 | +| New `python-multi-peer-add.md` | Task 5 | +| New `patterns-natural-key-preflight.md` | Task 6 | +| New `testing-integration.md` | Task 7 | +| Update `_sections.md` | Task 9 | +| New `graders/managing-generators/lib.py` with AST helpers | Tasks 1, 2 | +| `CHECKS` registry | Tasks 3-6 (append per rule) | +| `check_relationship_hfid_encoding.py` | Task 3 | +| `check_relationship_three_forms.py` | Task 4 | +| `check_multi_peer_iteration.py` | Task 5 | +| `check_natural_key_preflight.py` | Task 6 | +| Eval task: `generator-relationship-hfid-encoding` | Task 3 | +| Eval task: `generator-relationship-three-forms` | Task 4 | +| Eval task: `generator-multi-peer-iteration` | Task 5 | +| Eval task: `generator-natural-key-preflight` | Task 6 | +| Eval task: `generator-process-integration-test` (advisory) | Task 7 | +| Run `sync-evals.py` | Task 10 | +| Update `CHANGELOG.md` | Task 12 | +| Open draft PR | Task 13 | + +All spec items have at least one task. No gaps. + +**2. Placeholder scan:** None. Each task shows the complete code, +the exact command, and the expected output where applicable. The +only "decide based on results" step is Task 11 Step 3 (iterate on +smoke failures), which is properly framed as a conditional +follow-up, not a placeholder. + +**3. Type consistency:** + +- `CHECKS` is a `dict[str, Callable]` throughout. +- Assertion names are kebab-case strings (matching the existing + managing-schemas convention): `relationship-hfid-form-correct`, + `no-bare-string-relationship`, etc. Reused consistently in + `lib.py`, task graders, and `eval.yaml`. +- Helper function names are snake_case: `find_client_create_calls`, + `is_hfid_dict`, etc. Used consistently in tests and lib.py. +- Eval task names match between `eval.yaml` and the grader + script paths. + +No inconsistencies found. diff --git a/dev/plans/2026-05-19-managing-transforms-integration-rules.md b/dev/plans/2026-05-19-managing-transforms-integration-rules.md new file mode 100644 index 0000000..2182848 --- /dev/null +++ b/dev/plans/2026-05-19-managing-transforms-integration-rules.md @@ -0,0 +1,1610 @@ +# managing-transforms Integration-Layer Rules Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. +> Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the integration-layer gaps in +`infrahub-managing-transforms` that allowed two real-world demo +bugs — union-typed relationship queried as a non-union (bug 1), +and async artifact regeneration treated as synchronous (bug 7). + +**Architecture:** Two new rule files + one section-index edit + +new `graders/managing-transforms/` package with text-parsing +helpers for `.gql` files and AST helpers for `.py` files. Two +new eval tasks gated by deterministic graders. + +**Tech Stack:** Markdown (rules), Python 3.10+ stdlib `ast` +module (graders), regex-based GraphQL text helpers (full +`graphql-core` parsing is overkill for the checks needed here), +PyYAML, skillgrade, pytest. + +**Spec:** +[`dev/specs/2026-05-19-managing-transforms-integration-rules-design.md`](../specs/2026-05-19-managing-transforms-integration-rules-design.md) + +**Branch context:** This plan continues on the +`general-improvements` branch. PR 1 (managing-generators) is +already committed locally. PR 2 commits add to the same branch. +The user can split into separate PRs later by cherry-picking. + +--- + +## File Structure + +### New files + +```text +skills/infrahub-managing-transforms/rules/ +├── queries-union-fragments.md # new rule (CRITICAL) +└── artifacts-async-regen-polling.md # new rule (HIGH) + +graders/managing-transforms/ +├── __init__.py # empty package marker +├── lib.py # text + AST helpers + CHECKS +├── check_query_union_fragments.py # task grader +└── check_artifact_regen_polling.py # task grader + +tests/graders/ +└── test_transforms_lib.py # pytest tests for the new lib +``` + +### Modified files + +```text +skills/infrahub-managing-transforms/rules/ +└── _sections.md # register `queries-` prefix + +eval.yaml # add 2 tasks +evaluations/infrahub-managing-transforms.json # generated by sync-evals +CHANGELOG.md # extend [Unreleased] block +``` + +--- + +## Task 1: Scaffold grader package + +**Files:** + +- Create: `graders/managing-transforms/__init__.py` +- Create: `graders/managing-transforms/lib.py` + +Mirror the package shape used in `graders/managing-generators/` +(scaffolded in PR 1 task 1). Start with empty `__init__.py`, +module docstring + I/O helpers + empty `CHECKS` registry + +`run_checks` entry point. + +- [ ] **Step 1: Create the package marker** + +```bash +mkdir -p graders/managing-transforms +touch graders/managing-transforms/__init__.py +``` + +- [ ] **Step 2: Create lib.py skeleton** + +Create `graders/managing-transforms/lib.py`: + +```python +"""Shared grader library for infrahub-managing-transforms evaluations. + +Provides text-parsing helpers for ``.gql`` files, Python AST +helpers for ``.py`` files, individual check functions, a +``CHECKS`` registry, and the top-level ``run_checks`` entry +point that returns skillgrade JSON. + +Two output kinds are supported: + +- ``output.gql`` — raw GraphQL query text. The union-fragments + checks use simple regex/text matching rather than a full + GraphQL parser; this is fragile by design but cheap and + matches the failure shape we care about. +- ``output.py`` — Python source for the artifact-regen polling + eval. Checks use AST parsing. + +Usage (in a per-task grader script):: + + from pathlib import Path + from lib import run_checks + + result = run_checks( + ["query-uses-inline-fragments-for-location"], + {"gql": Path("output.gql")}, + ) +""" + +from __future__ import annotations + +import ast +from pathlib import Path +from typing import Any, Iterator + + +# --------------------------------------------------------------------------- +# I/O helpers +# --------------------------------------------------------------------------- + + +def load_output_gql(path: Path) -> str: + """Load a GraphQL query file. Returns empty string on missing file.""" + try: + return Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return "" + + +def load_output_py(path: Path) -> tuple[ast.Module | None, str]: + """Load a Python source file and return ``(parsed_tree, raw_text)``. + + Returns ``(None, "")`` if the file does not exist. + Returns ``(None, raw)`` if the file exists but has a syntax error. + """ + try: + raw = Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return None, "" + try: + tree = ast.parse(raw) + except SyntaxError: + return None, raw + return tree, raw + + +# --------------------------------------------------------------------------- +# CHECKS registry (populated by later tasks) +# --------------------------------------------------------------------------- + +CHECKS: dict[str, Any] = {} + + +# --------------------------------------------------------------------------- +# run_checks — top-level entry point +# --------------------------------------------------------------------------- + + +def run_checks( + check_names: list[str], + output_paths: dict[str, Path], +) -> dict: + """Run named checks against one or more output files. + + Parameters + ---------- + check_names: + List of assertion names from ``CHECKS``. + output_paths: + Mapping of output kind to path. Recognised keys: ``"gql"``, + ``"py"``. Each check function declares which input it + needs via ``**kwargs``. + + Returns skillgrade JSON ``{"score", "details", "checks"}``. + Raises ``KeyError`` if any check name is unknown. + """ + gql_text = load_output_gql(output_paths.get("gql", Path("output.gql"))) + tree, py_raw = load_output_py(output_paths.get("py", Path("output.py"))) + + entries: list[dict] = [] + passed_count = 0 + + for name in check_names: + fn = CHECKS[name] + try: + ok, msg = fn(gql_text=gql_text, tree=tree, py_raw=py_raw) + except Exception as exc: # defensive — never let one check crash all + ok, msg = False, f"Error running check: {exc}" + + if ok: + passed_count += 1 + entries.append({"name": name, "passed": ok, "message": msg}) + + total = len(check_names) + score = round(passed_count / total, 4) if total > 0 else 0.0 + + failed = [e["name"] for e in entries if not e["passed"]] + if failed: + details = f"{passed_count}/{total} checks passed. Failed: {', '.join(failed)}" + else: + details = f"All {total} checks passed." + + return {"score": score, "details": details, "checks": entries} +``` + +- [ ] **Step 3: Smoke-test the imports** + +```bash +python -c " +import importlib.util +from pathlib import Path +p = Path('graders/managing-transforms/lib.py') +spec = importlib.util.spec_from_file_location('m', p) +m = importlib.util.module_from_spec(spec) +spec.loader.exec_module(m) +print('CHECKS:', m.CHECKS) +print('run_checks empty:', m.run_checks([], {})) +" +``` + +Expected: + +```text +CHECKS: {} +run_checks empty: {'score': 0.0, 'details': 'All 0 checks passed.', 'checks': []} +``` + +- [ ] **Step 4: Commit** + +```bash +git add graders/managing-transforms/__init__.py graders/managing-transforms/lib.py +git commit -m "feat(graders): scaffold managing-transforms grader package" +``` + +--- + +## Task 2: Add helper functions (TDD) + +**Files:** + +- Modify: `graders/managing-transforms/lib.py` +- Create: `tests/graders/test_transforms_lib.py` + +Add the helpers that the check functions in Tasks 3-4 will use: + +- `find_inline_fragments(gql_text)` — returns list of fragment + type names appearing as `... on ` +- `block_for_relationship(gql_text, rel_name)` — extracts the + text of ` { ... }` so we can inspect what's inside +- `field_appears_directly_under_union(gql_text, rel_name, + field)` — heuristic: does the query select `` inside + a ` { node { ... } }` block *without* a preceding + `... on` fragment in that same node block? +- `has_post_to_artifact_generate(tree)` — True if any AST call + is `httpx.post`, `requests.post`, `client.post`, or + `self.client.post` with a URL argument string containing + `/api/artifact/generate` +- `has_loop_construct(tree)` — True if `ast.While` or + `ast.For` appears in the tree +- `references_core_artifact_in_call(tree)` — True if any + call passes `kind="CoreArtifact"` as a keyword argument +- [ ] **Step 1: Write the failing tests** + +Create `tests/graders/test_transforms_lib.py`: + +```python +"""Tests for graders/managing-transforms/lib.py helpers.""" + +import ast +import importlib.util +from pathlib import Path + + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_LIB_PATH = _REPO_ROOT / "graders" / "managing-transforms" / "lib.py" +_spec = importlib.util.spec_from_file_location( + "managing_transforms_graders_lib", _LIB_PATH +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + + +find_inline_fragments = _mod.find_inline_fragments +block_for_relationship = _mod.block_for_relationship +field_appears_directly_under_union = _mod.field_appears_directly_under_union +has_post_to_artifact_generate = _mod.has_post_to_artifact_generate +has_loop_construct = _mod.has_loop_construct +references_core_artifact_in_call = _mod.references_core_artifact_in_call + + +# -- find_inline_fragments ------------------------------------------------- + + +def test_find_inline_fragments_basic(): + gql = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + fragments = find_inline_fragments(gql) + assert "LocationSite" in fragments + assert "LocationBuilding" in fragments + + +def test_find_inline_fragments_empty(): + assert find_inline_fragments("") == [] + assert find_inline_fragments("query { foo { bar } }") == [] + + +# -- block_for_relationship ----------------------------------------------- + + +def test_block_for_relationship_extracts_braced_block(): + gql = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } } } + } + } + } +} +""" + block = block_for_relationship(gql, "location") + assert block is not None + assert "node" in block + assert "name" in block + + +def test_block_for_relationship_missing_returns_none(): + gql = "query { DcimDevice { edges { node { name { value } } } } }" + assert block_for_relationship(gql, "location") is None + + +# -- field_appears_directly_under_union ----------------------------------- + + +def test_field_directly_under_union_detects_bug(): + bug_gql = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } shortname { value } } } + } + } + } +} +""" + assert field_appears_directly_under_union(bug_gql, "location", "name") is True + assert field_appears_directly_under_union(bug_gql, "location", "shortname") is True + + +def test_field_directly_under_union_passes_with_fragments(): + good_gql = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + # name is inside a fragment, not directly under location > node + assert field_appears_directly_under_union(good_gql, "location", "name") is False + + +def test_field_directly_under_union_missing_relationship(): + gql = "query { DcimDevice { edges { node { name { value } } } } }" + assert field_appears_directly_under_union(gql, "location", "name") is False + + +# -- has_post_to_artifact_generate ---------------------------------------- + + +def test_has_post_to_artifact_generate_httpx(): + tree = ast.parse( + "import httpx\n" + "r = httpx.post('https://infrahub/api/artifact/generate/abc?branch=main')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_requests(): + tree = ast.parse( + "import requests\n" + "r = requests.post(f'/api/artifact/generate/{def_id}?branch={br}')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_self_client(): + tree = ast.parse( + "async def f(self):\n" + " await self.client.post('/api/artifact/generate/xyz?branch=dev')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_no_match(): + tree = ast.parse("import httpx\nr = httpx.post('/api/other')\n") + assert has_post_to_artifact_generate(tree) is False + + +# -- has_loop_construct --------------------------------------------------- + + +def test_has_loop_construct_while(): + tree = ast.parse("while True:\n pass\n") + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_for(): + tree = ast.parse("for i in range(10):\n pass\n") + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_async_for(): + tree = ast.parse( + "async def f():\n" + " async for x in stream():\n" + " pass\n" + ) + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_none(): + tree = ast.parse("x = 1\ny = 2\n") + assert has_loop_construct(tree) is False + + +# -- references_core_artifact_in_call ------------------------------------- + + +def test_references_core_artifact_filters_call(): + tree = ast.parse( + "x = client.filters(kind='CoreArtifact', definition__ids=[def_id])\n" + ) + assert references_core_artifact_in_call(tree) is True + + +def test_references_core_artifact_get_call(): + tree = ast.parse("x = client.get(kind='CoreArtifact', id=art_id)\n") + assert references_core_artifact_in_call(tree) is True + + +def test_references_core_artifact_no_match(): + tree = ast.parse("x = client.filters(kind='CoreNode')\n") + assert references_core_artifact_in_call(tree) is False +``` + +- [ ] **Step 2: Run tests to verify they fail** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v +``` + +Expected: all 16 tests fail with `AttributeError` (helpers not +yet defined). + +- [ ] **Step 3: Add helpers to lib.py** + +Append after the I/O helpers block (before `CHECKS = {}`): + +```python +# --------------------------------------------------------------------------- +# GraphQL text helpers +# --------------------------------------------------------------------------- + +import re + +_INLINE_FRAGMENT_RE = re.compile(r"\.\.\.\s*on\s+([A-Za-z_][A-Za-z0-9_]*)") + + +def find_inline_fragments(gql_text: str) -> list[str]: + """Return all type names appearing in ``... on ``.""" + return _INLINE_FRAGMENT_RE.findall(gql_text or "") + + +def _find_balanced_block(text: str, start: int) -> str | None: + """Given an index pointing at ``{``, return the substring up to + the matching ``}`` (inclusive). Returns ``None`` on imbalance. + """ + if start >= len(text) or text[start] != "{": + return None + depth = 0 + for i in range(start, len(text)): + ch = text[i] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return text[start : i + 1] + return None + + +def block_for_relationship(gql_text: str, rel_name: str) -> str | None: + """Return the text of `` { ... }`` (first occurrence).""" + pattern = re.compile(rf"\b{re.escape(rel_name)}\s*\{{") + match = pattern.search(gql_text or "") + if not match: + return None + return _find_balanced_block(gql_text, match.end() - 1) + + +def field_appears_directly_under_union( + gql_text: str, rel_name: str, field: str +) -> bool: + """Heuristic: does the query select ```` inside + `` { node { ... } }`` *without* a preceding + ``... on `` fragment in that same node block? + + Returns ``False`` if the relationship isn't queried at all, + or if the query uses inline fragments around the field. + """ + block = block_for_relationship(gql_text, rel_name) + if block is None: + return False + # Find the inner `node { ... }` block + node_match = re.search(r"\bnode\s*\{", block) + if not node_match: + return False + node_block = _find_balanced_block(block, node_match.end() - 1) + if node_block is None: + return False + # If there are inline fragments inside this node block, treat as safe. + if find_inline_fragments(node_block): + return False + # Otherwise, does the field appear as a direct sub-selection? + field_pattern = re.compile(rf"\b{re.escape(field)}\s*\{{") + return bool(field_pattern.search(node_block)) + + +# --------------------------------------------------------------------------- +# Python AST helpers +# --------------------------------------------------------------------------- + + +def _iter_calls(tree: ast.Module) -> Iterator[ast.Call]: + for node in ast.walk(tree): + if isinstance(node, ast.Call): + yield node + + +_ARTIFACT_GENERATE_PATH = "/api/artifact/generate" + + +def _string_contains(node: ast.AST, needle: str) -> bool: + """True if ``node`` is a string literal or f-string containing ``needle``.""" + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return needle in node.value + if isinstance(node, ast.JoinedStr): + return any( + isinstance(v, ast.Constant) and isinstance(v.value, str) and needle in v.value + for v in node.values + ) + return False + + +def has_post_to_artifact_generate(tree: ast.Module | None) -> bool: + """True if any ``post(...)`` call's URL argument mentions + ``/api/artifact/generate``. + + Accepts any callee whose attribute name is ``post`` — + ``httpx.post``, ``requests.post``, ``client.post``, + ``self.client.post``, etc. Inspects positional arg[0] and + keyword args ``url`` / ``path`` for the substring. + """ + if tree is None: + return False + for call in _iter_calls(tree): + func = call.func + if not (isinstance(func, ast.Attribute) and func.attr == "post"): + continue + candidates: list[ast.AST] = list(call.args[:1]) + for kw in call.keywords: + if kw.arg in ("url", "path", "endpoint"): + candidates.append(kw.value) + for c in candidates: + if _string_contains(c, _ARTIFACT_GENERATE_PATH): + return True + return False + + +def has_loop_construct(tree: ast.Module | None) -> bool: + """True if ``ast.While`` or ``ast.For`` (sync or async) is in tree.""" + if tree is None: + return False + for node in ast.walk(tree): + if isinstance(node, (ast.While, ast.For, ast.AsyncFor)): + return True + return False + + +def references_core_artifact_in_call(tree: ast.Module | None) -> bool: + """True if any call passes ``kind="CoreArtifact"`` as a keyword arg.""" + if tree is None: + return False + for call in _iter_calls(tree): + for kw in call.keywords: + if kw.arg == "kind" and isinstance(kw.value, ast.Constant): + if kw.value.value == "CoreArtifact": + return True + return False +``` + +Also: at the top of `lib.py`, ensure the import block has +`import re` near `import ast`. (Insert it if missing.) + +- [ ] **Step 4: Run tests to verify they pass** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v +``` + +Expected: all 16 tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add graders/managing-transforms/lib.py tests/graders/test_transforms_lib.py +git commit -m "feat(graders): add GraphQL + AST helpers for managing-transforms lib" +``` + +--- + +## Task 3: Union-fragments (rule + checks + grader + eval) + +**Files:** + +- Create: `skills/infrahub-managing-transforms/rules/queries-union-fragments.md` +- Modify: `graders/managing-transforms/lib.py` (add checks) +- Modify: `tests/graders/test_transforms_lib.py` (add check tests) +- Create: `graders/managing-transforms/check_query_union_fragments.py` +- Modify: `eval.yaml` + +Covers bug 1. + +- [ ] **Step 1: Write the rule file** + +Create `skills/infrahub-managing-transforms/rules/queries-union-fragments.md`: + +````markdown +--- +title: Querying Union-Typed Relationships with Inline Fragments +impact: CRITICAL +tags: gql, query, union, generic, inline-fragment, location +--- + +## Querying Union-Typed Relationships with Inline Fragments + +**Impact:** CRITICAL + +When a relationship's ``peer:`` in the schema is a **generic** +(or any type with multiple concrete inheritors that diverge in +fields), the GraphQL query must use inline fragments +(``... on TypeName { fields }``) to select fields that exist on +some inheritors but not others. Selecting such fields directly +on the union fails for any concrete instance whose type doesn't +define the field — the server returns +``Cannot query field 'X' on type 'Y'`` and the entire query +errors out. + +### Common union-typed relationships in Infrahub's base schema + +| Node.relationship | Peer (union) | Concrete inheritors | +| ----------------- | ------------ | ------------------- | +| ``DcimDevice.location`` | ``LocationGeneric`` | ``LocationSite``, ``LocationBuilding``, ``LocationHosting`` | +| ``OrganizationGeneric`` peers | ``OrganizationGeneric`` | ``OrganizationManufacturer``, ``OrganizationProvider``, ``OrganizationTenant`` | + +(Specifics vary by repo schema; always check ``peer:`` in +the schema before writing the query.) + +### Anti-pattern + +```graphql +query { + DcimDevice { + edges { + node { + location { + node { + name { value } # fails for LocationHosting + shortname { value } + } + } + } + } + } +} +``` + +Server response when the dataset contains a ``LocationHosting`` +instance: + +```text +Cannot query field 'name' on type 'LocationHosting'. +``` + +In a ``CoreRepository`` import, this stops the schema-sync; the +repo never finishes loading and downstream pipelines fail with +no obvious root cause. + +### Correct pattern + +```graphql +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + # LocationHosting has neither — explicitly skip, + # or include only fields it actually defines. + } + } + } + } + } +} +``` + +### How to know if a relationship needs fragments + +1. Find the relationship in the schema. Inspect ``peer:``. +2. If ``peer:`` is a generic type (or matches a ``generics:`` + entry by namespace+name), the relationship is a union. +3. Find every node with ``inherit_from:`` containing that + generic. Check whether each declares the same attribute + set. If they diverge, you need fragments. +4. When in doubt, use fragments. They never hurt; their + absence does. + +Reference: +[Infrahub schema docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Write the failing tests** + +Append to `tests/graders/test_transforms_lib.py`: + +```python +# -- Task 3: union-fragment checks ---------------------------------------- + + +SRC_BUG_QUERY = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } shortname { value } } } + } + } + } +} +""" + +SRC_GOOD_QUERY = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + + +def test_query_uses_inline_fragments_for_location_passes_on_good(): + ok, msg = _mod.CHECKS["query-uses-inline-fragments-for-location"]( + gql_text=SRC_GOOD_QUERY, tree=None, py_raw="" + ) + assert ok, msg + + +def test_query_uses_inline_fragments_for_location_fails_on_bug(): + ok, msg = _mod.CHECKS["query-uses-inline-fragments-for-location"]( + gql_text=SRC_BUG_QUERY, tree=None, py_raw="" + ) + assert not ok + + +def test_query_no_direct_field_on_union_location_passes_on_good(): + ok, msg = _mod.CHECKS["query-no-direct-field-on-union-location"]( + gql_text=SRC_GOOD_QUERY, tree=None, py_raw="" + ) + assert ok, msg + + +def test_query_no_direct_field_on_union_location_fails_on_bug(): + ok, msg = _mod.CHECKS["query-no-direct-field-on-union-location"]( + gql_text=SRC_BUG_QUERY, tree=None, py_raw="" + ) + assert not ok + assert "name" in msg or "shortname" in msg +``` + +- [ ] **Step 3: Run tests, confirm they fail** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v -k "fragments_for_location or direct_field" +``` + +Expected: 4 tests fail with `KeyError`. + +- [ ] **Step 4: Add check functions to lib.py** + +Append before the `CHECKS = {}` line: + +```python +# --------------------------------------------------------------------------- +# Union-fragments checks +# --------------------------------------------------------------------------- + +# Known union-typed relationships in Infrahub base schema. The +# grader is intentionally narrow — extend this dict when new +# unions enter the eval corpus. +_KNOWN_UNION_RELATIONSHIPS = { + "location": ("name", "shortname"), # rel name -> divergent fields +} + + +def check_query_uses_inline_fragments_for_location( + gql_text: str = "", **_: Any +) -> tuple[bool, str]: + """If the query touches ``location``, it must contain at least one + ``... on Location<...>`` inline fragment. + """ + if not gql_text: + return False, "No GraphQL output to inspect" + block = block_for_relationship(gql_text, "location") + if block is None: + return False, "Query does not touch 'location' relationship" + fragments = find_inline_fragments(block) + if any(f.startswith("Location") for f in fragments): + return True, f"Uses inline fragments: {fragments}" + return False, "location { ... } contains no ... on Location fragment" + + +def check_query_no_direct_field_on_union_location( + gql_text: str = "", **_: Any +) -> tuple[bool, str]: + """The query must not select ``name``/``shortname`` directly on + the ``location`` union (the bug 1 pattern). + """ + if not gql_text: + return False, "No GraphQL output to inspect" + bad: list[str] = [] + for field in _KNOWN_UNION_RELATIONSHIPS["location"]: + if field_appears_directly_under_union(gql_text, "location", field): + bad.append(field) + if bad: + return False, f"Direct field(s) on union location.node: {', '.join(bad)}" + return True, "No direct field selections on union location.node" +``` + +Update `CHECKS`: + +```python +CHECKS: dict[str, Any] = { + "query-uses-inline-fragments-for-location": check_query_uses_inline_fragments_for_location, + "query-no-direct-field-on-union-location": check_query_no_direct_field_on_union_location, +} +``` + +- [ ] **Step 5: Run tests, confirm they pass** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v +``` + +Expected: 20 passed (16 helper tests + 4 check tests). + +- [ ] **Step 6: Write task grader** + +Create `graders/managing-transforms/check_query_union_fragments.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the transform-query-union-fragments eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "query-uses-inline-fragments-for-location", + "query-no-direct-field-on-union-location", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, {"gql": Path("output.gql")}))) +``` + +- [ ] **Step 7: Verify the grader** + +```bash +rm -rf /tmp/grader-union; mkdir -p /tmp/grader-union/pass /tmp/grader-union/fail +cat > /tmp/grader-union/pass/output.gql <<'GQL' +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +GQL +cat > /tmp/grader-union/fail/output.gql <<'GQL' +query { + DcimDevice { + edges { + node { + location { node { name { value } shortname { value } } } + } + } + } +} +GQL +REPO=$(pwd) +cd /tmp/grader-union/pass && python "$REPO/graders/managing-transforms/check_query_union_fragments.py" +echo "---" +cd /tmp/grader-union/fail && python "$REPO/graders/managing-transforms/check_query_union_fragments.py" +cd "$REPO" +``` + +Expected: pass=1.0, fail=0.0. + +- [ ] **Step 8: Add eval task to eval.yaml** + +Append to the `tasks:` list (after the existing managing-generators tasks): + +```yaml + # --- Transforms: union-typed relationship queries --- # + - name: transform-query-union-fragments + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-transforms/SKILL.md and + follow its workflow and rules. + + Task: Write a GraphQL query for a Jinja2 transform that fetches DcimDevice + with the device's name, its role, and its location's name + shortname. + location is a union of LocationSite | LocationBuilding | LocationHosting + in this repo's base schema. LocationHosting has neither name nor shortname, + so the query must handle the union explicitly so that schema-sync doesn't + fail when a LocationHosting instance is present. + + Save ONLY the final GraphQL query to: output.gql + graders: + - type: deterministic + run: python graders/managing-transforms/check_query_union_fragments.py + weight: 1.0 + expected_output: >- + A GraphQL query that uses inline fragments (... on LocationSite { ... } + and similar) for the location relationship, so fields that exist on + some concrete types but not others are scoped to the correct type. + expectations: + - "Query touches DcimDevice and location" + - "Inline fragments (... on Location...) are used inside location.node" + - "name and shortname are not selected directly under location.node" + assertions: + - name: query-uses-inline-fragments-for-location + check: >- + location { node { ... } } contains at least one ... on Location + inline fragment + - name: query-no-direct-field-on-union-location + check: >- + Neither 'name' nor 'shortname' appears as a direct selection under + location.node without an enclosing inline fragment +``` + +- [ ] **Step 9: Commit** + +```bash +git add \ + skills/infrahub-managing-transforms/rules/queries-union-fragments.md \ + graders/managing-transforms/lib.py \ + graders/managing-transforms/check_query_union_fragments.py \ + tests/graders/test_transforms_lib.py \ + eval.yaml +git commit -m "feat(transforms): rule + grader for union-relationship query fragments" +``` + +--- + +## Task 4: Artifact regen polling (rule + checks + grader + eval) + +**Files:** + +- Create: `skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md` +- Modify: `graders/managing-transforms/lib.py` +- Modify: `tests/graders/test_transforms_lib.py` +- Create: `graders/managing-transforms/check_artifact_regen_polling.py` +- Modify: `eval.yaml` + +Covers bug 7. + +- [ ] **Step 1: Write the rule file** + +Create `skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md`: + +````markdown +--- +title: Programmatic Artifact Regeneration Is Fire-and-Forget +impact: HIGH +tags: artifacts, regenerate, async, polling, CoreArtifact +--- + +## Programmatic Artifact Regeneration Is Fire-and-Forget + +**Impact:** HIGH + +``POST /api/artifact/generate/?branch=`` returns +HTTP 200 as soon as the regen request is **queued**, not when +regen is **finished**. The async server-side regen can run +seconds later, and on a busy system can even run before a +recent generator's group-membership write is visible on the +read replica — at which point the regen sees the wrong target +set and silently produces nothing. + +Any caller that triggers regen programmatically (catalog page, +CI job, generator orchestration) **must poll** ``CoreArtifact`` +until the expected count materialises, with a hard timeout that +surfaces a warning instead of silently shipping zero artifacts. + +### Anti-pattern + +```python +async def regenerate(client, def_id, branch): + # WRONG — fire-and-forget; "success" doesn't mean anything finished + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + return "regenerated" +``` + +### Correct pattern + +```python +import asyncio + +POLL_INTERVAL_SECONDS = 2 +TIMEOUT_SECONDS = 60 + + +async def regenerate_and_wait(client, def_id, expected_count, branch): + """Trigger regen, poll until artifact count matches, or warn on timeout.""" + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + + deadline = asyncio.get_event_loop().time() + TIMEOUT_SECONDS + reposted = False + while asyncio.get_event_loop().time() < deadline: + artifacts = await client.filters( + kind="CoreArtifact", + definition__ids=[def_id], + branch=branch, + ) + if len(artifacts) >= expected_count: + return artifacts + if not reposted: + # Re-POST once: covers the read-replica visibility gap + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + reposted = True + await asyncio.sleep(POLL_INTERVAL_SECONDS) + + raise TimeoutError( + f"Artifact regen for {def_id} did not converge " + f"to {expected_count} within {TIMEOUT_SECONDS}s" + ) +``` + +### When this matters + +- **Catalog/UI pages** that trigger regen on user action — the + user clicks "deploy" and walks away; silent failure is the + worst outcome. +- **CI jobs** that gate a deploy on regen success. +- **Orchestrators** that chain generator → regen → assertion. + +Manual regen in the Infrahub UI doesn't need this — the UI +polls on its own. + +Reference: +[Infrahub artifacts docs](https://docs.infrahub.app) +```` + +- [ ] **Step 2: Write failing check tests** + +Append to `tests/graders/test_transforms_lib.py`: + +```python +# -- Task 4: artifact regen polling checks -------------------------------- + + +SRC_GOOD_POLLING = """ +import asyncio + + +async def regen_and_wait(client, def_id, expected_count, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + deadline = asyncio.get_event_loop().time() + 60 + while asyncio.get_event_loop().time() < deadline: + artifacts = await client.filters( + kind="CoreArtifact", definition__ids=[def_id], branch=branch, + ) + if len(artifacts) >= expected_count: + return artifacts + await asyncio.sleep(2) + raise TimeoutError("regen did not converge") +""" + +SRC_BAD_FIRE_AND_FORGET = """ +async def regenerate(client, def_id, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + return "regenerated" +""" + +SRC_BAD_NO_POST = """ +async def maybe_regen(client, def_id): + artifacts = await client.filters(kind="CoreArtifact", definition__ids=[def_id]) + return artifacts +""" + + +def test_posts_artifact_generate_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["posts-artifact-generate-endpoint"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_posts_artifact_generate_fails_when_missing(): + tree = ast.parse(SRC_BAD_NO_POST) + ok, msg = _mod.CHECKS["posts-artifact-generate-endpoint"]( + gql_text="", tree=tree, py_raw=SRC_BAD_NO_POST + ) + assert not ok + + +def test_has_polling_loop_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["has-polling-loop"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_has_polling_loop_fails_on_fire_and_forget(): + tree = ast.parse(SRC_BAD_FIRE_AND_FORGET) + ok, msg = _mod.CHECKS["has-polling-loop"]( + gql_text="", tree=tree, py_raw=SRC_BAD_FIRE_AND_FORGET + ) + assert not ok + + +def test_polls_coreartifact_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["polls-coreartifact-after-post"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_polls_coreartifact_fails_on_fire_and_forget(): + tree = ast.parse(SRC_BAD_FIRE_AND_FORGET) + ok, msg = _mod.CHECKS["polls-coreartifact-after-post"]( + gql_text="", tree=tree, py_raw=SRC_BAD_FIRE_AND_FORGET + ) + assert not ok +``` + +- [ ] **Step 3: Run tests, confirm they fail** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v -k "polling or artifact_generate or coreartifact" +``` + +Expected: 6 tests fail with `KeyError`. + +- [ ] **Step 4: Add check functions** + +Append to `graders/managing-transforms/lib.py` (after the +union-fragment checks, before `CHECKS = {...}`): + +```python +# --------------------------------------------------------------------------- +# Artifact regen polling checks +# --------------------------------------------------------------------------- + + +def check_posts_artifact_generate_endpoint( + tree: ast.Module | None = None, **_: Any +) -> tuple[bool, str]: + """Source must contain a POST whose URL mentions /api/artifact/generate.""" + if tree is None: + return False, "No Python source to inspect" + if has_post_to_artifact_generate(tree): + return True, "POST to /api/artifact/generate found" + return False, "No POST to /api/artifact/generate found" + + +def check_has_polling_loop( + tree: ast.Module | None = None, **_: Any +) -> tuple[bool, str]: + """Source must contain at least one ``while``/``for``/``async for`` loop.""" + if tree is None: + return False, "No Python source to inspect" + if has_loop_construct(tree): + return True, "Loop construct found" + return False, "No loop construct found — fire-and-forget pattern" + + +def check_polls_coreartifact_after_post( + tree: ast.Module | None = None, **_: Any +) -> tuple[bool, str]: + """Source must reference ``kind="CoreArtifact"`` in a call (a read).""" + if tree is None: + return False, "No Python source to inspect" + if not has_post_to_artifact_generate(tree): + return False, "No POST to /api/artifact/generate; nothing to poll" + if references_core_artifact_in_call(tree): + return True, "CoreArtifact read found after POST" + return False, "No call references kind='CoreArtifact'" +``` + +Update CHECKS: + +```python +CHECKS: dict[str, Any] = { + "query-uses-inline-fragments-for-location": check_query_uses_inline_fragments_for_location, + "query-no-direct-field-on-union-location": check_query_no_direct_field_on_union_location, + "posts-artifact-generate-endpoint": check_posts_artifact_generate_endpoint, + "has-polling-loop": check_has_polling_loop, + "polls-coreartifact-after-post": check_polls_coreartifact_after_post, +} +``` + +- [ ] **Step 5: Run tests, confirm they pass** + +```bash +uvx --with pyyaml --with pytest pytest tests/graders/test_transforms_lib.py -v +``` + +Expected: 26 passed (20 from prior tasks + 6 new). + +- [ ] **Step 6: Write task grader** + +Create `graders/managing-transforms/check_artifact_regen_polling.py`: + +```python +#!/usr/bin/env python3 +"""Grader for the transform-artifact-regen-polling eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "posts-artifact-generate-endpoint", + "has-polling-loop", + "polls-coreartifact-after-post", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, {"py": Path("output.py")}))) +``` + +- [ ] **Step 7: Verify the grader** + +```bash +rm -rf /tmp/grader-poll; mkdir -p /tmp/grader-poll/pass /tmp/grader-poll/fail +cat > /tmp/grader-poll/pass/output.py <<'PY' +import asyncio + + +async def regen_and_wait(client, def_id, expected_count, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + deadline = asyncio.get_event_loop().time() + 60 + while asyncio.get_event_loop().time() < deadline: + artifacts = await client.filters( + kind="CoreArtifact", definition__ids=[def_id], branch=branch + ) + if len(artifacts) >= expected_count: + return artifacts + await asyncio.sleep(2) + raise TimeoutError("regen did not converge") +PY +cat > /tmp/grader-poll/fail/output.py <<'PY' +async def regenerate(client, def_id, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + return "regenerated" +PY +REPO=$(pwd) +cd /tmp/grader-poll/pass && python "$REPO/graders/managing-transforms/check_artifact_regen_polling.py" +echo "---" +cd /tmp/grader-poll/fail && python "$REPO/graders/managing-transforms/check_artifact_regen_polling.py" +cd "$REPO" +``` + +Expected: pass=1.0, fail < 1.0 (specifically: passes +`posts-artifact-generate-endpoint`, fails the other two). + +- [ ] **Step 8: Add eval task** + +Append to `eval.yaml`: + +```yaml + - name: transform-artifact-regen-polling + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-transforms/SKILL.md and + follow its workflow and rules. + + Task: Write a Python async helper named regenerate_and_wait that triggers + regeneration for an Infrahub artifact definition by POSTing to + /api/artifact/generate/?branch=, then waits for completion + by polling CoreArtifact filtered by the definition until at least + `expected_count` instances exist. Time out after 60 seconds and raise + TimeoutError. The helper takes (client, def_id, expected_count, branch) + as parameters. + + Save ONLY the final Python helper to: output.py + graders: + - type: deterministic + run: python graders/managing-transforms/check_artifact_regen_polling.py + weight: 1.0 + expected_output: >- + A Python async function that POSTs to /api/artifact/generate/, + then polls CoreArtifact in a loop until count >= expected_count, with + a hard timeout. + expectations: + - "Helper POSTs to /api/artifact/generate/?branch=" + - "Helper contains a loop (while or for) that waits for completion" + - "Helper polls CoreArtifact via client.filters or client.get with kind='CoreArtifact'" + - "Hard timeout exists (raises after some seconds)" + assertions: + - name: posts-artifact-generate-endpoint + check: >- + The code calls .post(...) with a URL containing + /api/artifact/generate + - name: has-polling-loop + check: >- + A while/for/async for loop is present (not fire-and-forget) + - name: polls-coreartifact-after-post + check: >- + A call passes kind="CoreArtifact" (read of artifact state + after the POST) +``` + +- [ ] **Step 9: Commit** + +```bash +git add \ + skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md \ + graders/managing-transforms/lib.py \ + graders/managing-transforms/check_artifact_regen_polling.py \ + tests/graders/test_transforms_lib.py \ + eval.yaml +git commit -m "feat(transforms): rule + grader for async artifact regen polling" +``` + +--- + +## Task 5: Update _sections.md + +**File:** `skills/infrahub-managing-transforms/rules/_sections.md` + +Add the new `queries-` prefix and extend the `artifacts-` +entry. + +- [ ] **Step 1: Read the current file** + +```bash +cat skills/infrahub-managing-transforms/rules/_sections.md +``` + +- [ ] **Step 2: Replace the relevant entries** + +The existing `5. **Artifacts (artifacts-)**` entry becomes: + +```markdown +5. **Artifacts (artifacts-)** -- HIGH. Connecting + transforms to output files via artifact_definitions, + content types, targets (CoreArtifactTarget), + parameter mapping, and async regeneration polling + (the /api/artifact/generate endpoint is fire-and-forget). +``` + +Add a new section entry between Jinja2 (item 3) and Hybrid +(item 4) — renumber if needed — or place it as the last entry, +matching the conventions used in `_sections.md`. The exact slot +is at the implementer's judgement; the entry text is: + +```markdown +9. **Queries (queries-)** -- CRITICAL. Writing the .gql + query that feeds the transform. Covers union-typed + relationships (DcimDevice.location, Organization*) that + require inline fragments (... on Type { fields }) to + avoid "Cannot query field 'X' on type 'Y'" errors. +``` + +If renumbering breaks the existing order, just append as the +next available number (the categories aren't strictly ordered; +they're indexed by prefix not number). + +- [ ] **Step 3: Commit** + +```bash +git add skills/infrahub-managing-transforms/rules/_sections.md +git commit -m "docs(transforms): register queries- prefix and extend artifacts-" +``` + +--- + +## Task 6: Regenerate evaluations JSON + +**Files:** + +- Generated: `evaluations/infrahub-managing-transforms.json` + +- [ ] **Step 1: Run sync-evals** + +```bash +python scripts/sync-evals.py +``` + +Expected output mentions managing-transforms with 2 evals. + +- [ ] **Step 2: Commit** + +```bash +git add evaluations/infrahub-managing-transforms.json +git commit -m "chore(evals): add managing-transforms JSON projection" +``` + +--- + +## Task 7: Update CHANGELOG.md + +**File:** `CHANGELOG.md` + +Add a new sub-section under `[Unreleased]` for transforms. + +- [ ] **Step 1: Edit CHANGELOG.md** + +Below the existing `### managing-generators` block in the +`[Unreleased]` section, add: + +```markdown +### managing-transforms + +- New rule `queries-union-fragments` documenting the need for + GraphQL inline fragments on union-typed relationships + (DcimDevice.location, Organization* peers) to avoid + "Cannot query field 'X' on type 'Y'" failures in + CoreRepository schema-sync. +- New rule `artifacts-async-regen-polling` documenting that + `POST /api/artifact/generate` is fire-and-forget and + programmatic callers must poll `CoreArtifact` to confirm + completion. +- Added 2 deterministic evals + graders to `eval.yaml` (first + evals for this skill). +``` + +- [ ] **Step 2: Commit** + +```bash +git add CHANGELOG.md +git commit -m "docs: changelog entries for managing-transforms improvements" +``` + +--- + +## Task 8: Final verification + +**Files:** none directly — verification only. + +- [ ] **Step 1: Run the full test suite** + +```bash +uvx --with pyyaml --with pytest pytest tests/ -v +``` + +Expected: all tests pass (179 from PR 1 + 26 new = 205). + +- [ ] **Step 2: Run lint** + +```bash +uvx rumdl check skills/infrahub-managing-transforms/ +``` + +Expected: no issues. + +- [ ] **Step 3: Confirm eval.yaml parses and `sync-evals.py` is + idempotent** + +```bash +python -c "import yaml; d=yaml.safe_load(open('eval.yaml')); print(f'tasks: {len(d[\"tasks\"])}')" +python scripts/sync-evals.py +git status --short +``` + +Expected: tasks: 19; sync-evals reports no changes (idempotent); +git status is clean. + +- [ ] **Step 4: Review the PR 2 commit range** + +```bash +git log ..HEAD --oneline +``` + +Where `` is the last PR 1 commit (`c1531e2`). Expect +roughly 7-8 commits for PR 2. + +No new commit in this task — verification only. + +--- + +## Self-Review + +**1. Spec coverage:** + +| Spec item | Task | +| --- | --- | +| New `queries-union-fragments.md` | Task 3 | +| New `artifacts-async-regen-polling.md` | Task 4 | +| Edit `_sections.md` | Task 5 | +| New `graders/managing-transforms/__init__.py` | Task 1 | +| New `graders/managing-transforms/lib.py` | Tasks 1-4 | +| 2 task grader scripts | Tasks 3, 4 | +| 2 eval tasks in `eval.yaml` with `trials: 3` | Tasks 3, 4 | +| `evaluations/infrahub-managing-transforms.json` | Task 6 | +| `CHANGELOG.md` update | Task 7 | + +All spec items have at least one task. + +**2. Placeholder scan:** None. All code blocks contain the +actual content. The only judgement call is the `_sections.md` +section number — explicitly noted as "implementer's judgement, +slot doesn't matter." + +**3. Type consistency:** + +- `CHECKS` is `dict[str, Callable]` and check functions accept + `gql_text=""`, `tree=None`, `py_raw=""` kwargs. +- Assertion names are kebab-case strings consistent across + `lib.py`, task graders, and `eval.yaml`. +- Helper names are snake_case: `find_inline_fragments`, + `block_for_relationship`, `field_appears_directly_under_union`, + `has_post_to_artifact_generate`, `has_loop_construct`, + `references_core_artifact_in_call`. +- `run_checks` signature `(check_names, output_paths)` is + consistent: `output_paths` is a `dict[str, Path]` with keys + `"gql"` and `"py"`. +- The check functions in CHECKS take `(gql_text, tree, py_raw)` + via kwargs, consumed appropriately by each check. + +No inconsistencies. diff --git a/dev/specs/2026-05-18-managing-generators-integration-rules-design.md b/dev/specs/2026-05-18-managing-generators-integration-rules-design.md new file mode 100644 index 0000000..b621a77 --- /dev/null +++ b/dev/specs/2026-05-18-managing-generators-integration-rules-design.md @@ -0,0 +1,279 @@ +# PR 1: managing-generators — integration-layer rules + evals + +**Date:** 2026-05-18 +**Branch:** `general-improvements` +**Source:** Real-world demo session feedback (7 bugs across one +Infrahub demo build, mapped to skill gaps) + +## Background + +Pete ran a multi-day session building an Infrahub SDWAN demo and +hit seven bugs that cost significant time. Four of them — bugs +2, 3, 4, and 5 in his debrief — live at the +`client.create` / `RelationshipManager` API surface. That is the +*integration layer* between what the model assumes about +Infrahub's SDK and what the server actually accepts. + +The `infrahub-managing-generators` skill currently covers +schema-layer concerns (architecture, idempotency, tracking) well, +but is thin on integration-layer detail. Worse, the existing +`python-generate.md` example uses misleading shorthand +(`'Use ID for relationships'`, `device_type_id` as a singular +variable with no shape hint) that directly contributed to bug 3. + +This PR closes that gap with four new rules, one rule edit, and +five eval tasks gated by AST-based deterministic graders. + +## Scope + +This PR scopes only `infrahub-managing-generators`. PRs 2-4 +(managing-transforms, infrahub-common, hook activation) are +tracked separately. + +## Bug coverage + +| Bug | Description | Rule covering it | +| --- | --- | --- | +| 2 | Composite HFID over-packed for single-component target | rule #2 (`python-relationship-references`) | +| 3 | Bare string interpreted as `id` lookup | rule #2 + rule #1 edit | +| 4 | List passed to `RelationshipManager.add` instead of iterating | rule #3 (`python-multi-peer-add`) | +| 5 | `IpamPrefix` uniqueness collision on bootstrap-seeded key | rule #4 (`patterns-natural-key-preflight`) | +| Cross-cutting | "Run generator end-to-end before merge" | rule #5 (`testing-integration`) | + +## Changes + +### Rule files + +1. **Edit** `skills/infrahub-managing-generators/rules/python-generate.md`: + - Replace the misleading `device_type_id` example with explicit + `{"hfid": [""]}` form. + - Remove the standalone comment `# Use ID for relationships`. + - Add a cross-reference pointer to new rule #2. + +2. **New** `skills/infrahub-managing-generators/rules/python-relationship-references.md` + (CRITICAL): + - The three accepted forms for relationship fields in + `client.create`: + - **HFID lookup (single-component):** + `{"hfid": [""]}` + - **HFID lookup (composite):** + `{"hfid": ["", ""]}` — list length must + match the schema's `human_friendly_id` declaration + - **Explicit ID:** `{"id": ""}` + - **SDK object reference:** pass a variable holding a node + previously returned by `client.get` or `client.create` + - **WARNING block:** a bare string for a relationship field is + interpreted as `{"id": ""}` — never HFID lookup. The + server returns "Unable to find the node" because the string + is not a valid UUID. + - **WARNING block:** over-packing the HFID list (e.g. passing + `["name", "manufacturer_name"]` for a single-component HFID) + fails. Inspect the schema's `human_friendly_id` value before + constructing the list. + - Cross-link to + `infrahub-managing-schemas/rules/display-human-friendly-id`. + +3. **New** `skills/infrahub-managing-generators/rules/python-multi-peer-add.md` + (HIGH): + - `RelationshipManager.add()` takes one peer per call. + - Passing a list literal (`group.members.add([p1, p2, p3])`) + creates ONE peer with a composite HFID equal to the list + contents. The resulting mutation looks like + `members: [{hfid: [, , ]}]` — wrong. + - Pattern: iterate the peer collection, call `.add(p)` once + per peer, then `.save()` once at the end. + - Same constraint applies to other RelationshipManager + mutators (`.update`, `.remove`). + +4. **New** `skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md` + (MEDIUM): + - When mutations are driven by user input (forms, catalogs, + ad-hoc scripts) and bootstrap data may have already seeded + the natural key, pre-flight check before creating. + - **Pattern A — pre-flight check** (`client.get` + branch on + `NodeNotFound`): use when the desired behavior is "fail + loudly with a friendly message if the object already + exists." + - **Pattern B — upsert** (`save(allow_upsert=True)`): use when + the desired behavior is "create or update silently." This + is the default in `InfrahubGenerator.generate()`. + - **Anti-pattern:** `client.create` followed by `.save()` with + no upsert and no preflight → raw server-side + `UniquenessConstraintError` reaches the user. + - Code skeletons for both patterns. + +5. **New** `skills/infrahub-managing-generators/rules/testing-integration.md` + (LOW, advisory): + - After implementing a generator, run it end-to-end against a + live Infrahub instance before declaring done. + - Unit tests on the input dict do not cover SDK call shape — + bugs 2, 3, and 4 all type-check and pass unit tests but + fail at runtime against the real server. + - Concrete workflow: `infrahubctl generator list` → + `infrahubctl generator run ` → verify created + objects exist in the UI or via GraphQL. + - Same workflow applies during PR review on a branch. + +6. **Edit** `skills/infrahub-managing-generators/rules/_sections.md`: + - Note `python-` now includes relationship-references and + multi-peer-add. + - Note `patterns-` now includes natural-key-preflight. + - Note `testing-` now includes integration. + +### Grader infrastructure + +7. **New** `graders/managing-generators/lib.py`: + - `load_output_py(path)` — parse a Python file via `ast`, + return module tree. Returns `(None, "")` on parse failure. + - `find_client_create_calls(tree)` — yield each + `await self.client.create(...)` call with parsed `kind` + string and the `data` dict literal as a Python dict (only + literal-resolvable values; opaque expressions are kept as + `ast` nodes). + - `find_relationship_manager_add_calls(tree)` — yield each + `.add(...)` call on attribute chains ending in `members`, + `peers`, or any attribute, with the argument's ast node. + - `is_hfid_dict_literal(node)` — + `{"hfid": []}` check. + - `is_bare_string_literal(node)` — `ast.Constant(value=str)`. + - `CHECKS` registry mapping assertion names to check + functions returning `(bool, str)`. + - `run_checks(checks, paths)` — return skillgrade JSON + (`{"score": float, "details": str, "checks": [...]}`). + +8. **New** `graders/managing-generators/check_relationship_hfid_encoding.py` + — task 1 grader. + +9. **New** `graders/managing-generators/check_relationship_three_forms.py` + — task 2 grader. + +10. **New** `graders/managing-generators/check_multi_peer_iteration.py` + — task 3 grader. + +11. **New** `graders/managing-generators/check_natural_key_preflight.py` + — task 4 grader. + +### Eval tasks (added to root `eval.yaml`) + +12. **Task** `generator-relationship-hfid-encoding` (rule #2): + - Prompt: build a POP topology generator that creates + DcimDevice instances referencing `device_type` and + `manufacturer` by HFID. Both are single-component HFIDs. + - Assertions: `relationship-hfid-form-correct`, + `no-bare-string-relationship`, `no-overpacked-hfid-list`. + +13. **Task** `generator-relationship-three-forms` (rule #2): + - Prompt: generator references nodes by all three forms + (HFID, SDK object from earlier `client.get`, explicit ID + from a query result). Forces the model to exercise the + full reference matrix. + - Assertions: `hfid-form-for-name-lookup`, + `sdk-object-reference-used`, `id-form-for-uuid`. + +14. **Task** `generator-multi-peer-iteration` (rule #3): + - Prompt: generator creates a `CoreStandardGroup` and adds 5 + devices to its `members`. + - Assertions: `members-add-iterates`, + `no-list-passed-to-add`. + +15. **Task** `generator-natural-key-preflight` (rule #4): + - Prompt: write a script that creates an `IpamPrefix` + `10.250.10.0/24` from user input. The prefix may already + exist from bootstrap data; handle the collision + gracefully. + - Assertions: `preflight-or-upsert`, + `no-raw-create-without-handler`. + +16. **Task** `generator-process-integration-test` (rule #5, + advisory): + - Prompt: standard generator request, plus "explain how to + test this end-to-end." + - **Expectations-only** (no grader). Reviewer checks that + the response mentions `infrahubctl generator run` against + a live instance, and that input-dict unit tests are + insufficient. + +### Sync + docs + +17. Run `python scripts/sync-evals.py` to regenerate + `evaluations/managing-generators.json`. Commit alongside the + `eval.yaml` change. + +18. Update `CHANGELOG.md` with a bulleted list of the rule + additions under the next version section. + +19. **Do not** bump skill version in this PR. Version bumps are + handled by the release workflow. + +## Implementation order + +1. Write 4 new rule files + edit `python-generate.md` + + update `_sections.md`. +2. Build `graders/managing-generators/lib.py` with AST helpers + and `CHECKS` registry. +3. Write 4 grader scripts. +4. Add 5 eval tasks to `eval.yaml`. +5. Run `python scripts/sync-evals.py`. +6. Verify each grader locally against hand-crafted compliant + + violating Python fixtures. A grader that returns `pass: true` + on a violating fixture is broken — fix before relying on + smoke. +7. Run `skillgrade --smoke`. If any rule fails reliably (model + ignores the prose), iterate on the rule: + - Strengthen the WARNING block. + - Add a concrete example of the failure mode. + - Cross-check the description in SKILL.md for activation + hints. +8. Update `CHANGELOG.md`. +9. Open **draft** PR (per global instruction; pass `--draft`). + +## Risks and mitigations + +- **AST grading complexity.** Existing graders parse YAML; this + PR introduces Python AST parsing. Mitigation: build `lib.py` + helpers carefully, hand-craft fixtures, run graders standalone + before trusting smoke. +- **Rule prose may not be strong enough.** Smoke may reveal that + the model still bare-strings relationships despite the new + rule. Mitigation: iterate on the rule prose, especially the + WARNING/example pairing. +- **CI matrix coverage.** This adds the first eval tasks for + `infrahub-managing-generators`. CI may need a config check to + confirm the matrix covers managing-generators. Investigate + before final push. +- **AST literal resolution.** `data={"name": var}` cannot be + fully resolved from AST alone (the value is a variable). The + graders should: + - Always fail if a relationship field is a bare string literal + (`ast.Constant(value=str)`) — this is the bug 3 pattern. + - Always fail if a relationship field is an over-packed list + literal for a single-component HFID. + - Treat opaque expressions (variable refs, function calls) as + "indeterminate" / pass — they may be valid SDK objects. + - Document this limitation in `lib.py`. + +## Out of scope + +- PR 2 (managing-transforms) — bugs 1 + 7 +- PR 3 (infrahub-common) — pre-merge GraphQL dry-run rule +- PR 4 (hook) — stronger advisory activation wording +- Open issues #22-25 (menu skill bugs) and #26-29 (other + generator concerns) — none are addressed by this PR + +## Acceptance + +PR 1 is done when: + +- All 4 new rule files + edited `python-generate.md` + updated + `_sections.md` are committed. +- `graders/managing-generators/` exists with `lib.py` + 4 task + graders. +- 5 new tasks are added to `eval.yaml`. +- `evaluations/managing-generators.json` is regenerated and + committed. +- Each grader returns the correct result on hand-crafted + fixtures (compliant + violating). +- `skillgrade --smoke` runs without crashing. (Pass rate goals + set after first smoke baseline.) +- `CHANGELOG.md` updated. +- Draft PR opened and self-reviewed. diff --git a/dev/specs/2026-05-19-managing-transforms-integration-rules-design.md b/dev/specs/2026-05-19-managing-transforms-integration-rules-design.md new file mode 100644 index 0000000..9c20c0d --- /dev/null +++ b/dev/specs/2026-05-19-managing-transforms-integration-rules-design.md @@ -0,0 +1,243 @@ +# PR 2: managing-transforms — union-fragments + async artifact regen + +**Date:** 2026-05-19 +**Branch:** `general-improvements` (continues after PR 1) +**Source:** Real-world demo session feedback (bugs 1, 7 of 7) + +## Background + +This is PR 2 of 4 in the managing-skills improvements arc. PR 1 +(closed) covered bugs 2-5 in `managing-generators`. PR 2 covers +the two transform-layer bugs from the same demo: + +- **Bug 1:** `queries/config/sdwan_edge.gql` queried + `DcimDevice.location` as a non-union — but `location` is + actually a union of `LocationGeneric | LocationSite | + LocationHosting | …`, and `LocationHosting` has no `name` + field. Result: `CoreRepository` schema-sync threw + `Cannot query field 'name' on type 'LocationHosting'`. The + repo never finished syncing → 0 generator definitions → + `invoke init` timed out. +- **Bug 7:** The catalog POSTs + `/api/artifact/generate/?branch=` for every + artifact definition. This endpoint is **fire-and-forget** + (returns 200 immediately). The async server-side regen + sometimes runs before the generator's group-membership write + is visible on the read replica → sees zero targets → silently + creates no artifacts. The catalog reported "success" while + shipping nothing. + +The `infrahub-managing-transforms` skill currently has rules for +type selection, Python/Jinja2 mechanics, and artifact +*configuration* — but says nothing about querying union +relationships, and nothing about the async nature of artifact +regeneration. This PR closes both gaps. + +## Scope + +PR 2 covers only `infrahub-managing-transforms`. PRs 3 and 4 +are tracked separately: + +- PR 3: cross-cutting GraphQL dry-run rule in `infrahub-common` +- PR 4: hook advisory wording + +## Bug coverage + +| Bug | Description | Rule | +| --- | --- | --- | +| 1 | Query field on union type (`DcimDevice.location` → `LocationHosting` has no `name`) | rule #1 (`queries-union-fragments`) | +| 7 | Artifact regen race (`/api/artifact/generate` is fire-and-forget) | rule #2 (`artifacts-async-regen-polling`) | + +## Changes + +### Rule files + +1. **New** `skills/infrahub-managing-transforms/rules/queries-union-fragments.md` + (CRITICAL): + - When a relationship's `peer:` in the schema is a **generic** + (or a union of generics), the query must use GraphQL inline + fragments (`... on TypeName { fields }`) to select fields + that are specific to each concrete type. + - Querying fields directly on the union (e.g., + `location { node { name { value } } }`) fails for any + concrete type that doesn't define that field. + - List of the most common union-typed relationships in + Infrahub's base schema (table of `node.relationship → peer + generic`), with the concrete types that inherit from each + and which fields they all share vs. which are type-specific. + - **Anti-pattern** showing the bug 1 query verbatim and the + server error. + - **Correct pattern** using `... on LocationSite { … } ... on + LocationBuilding { … }` (and explicit handling for types + with no useful field, like `LocationHosting`). + - "How to know if a relationship is a union" — find the + `peer:` in the schema; if it's a generic name and the + generic has `inherit_from` consumers with diverging + attribute sets, you need fragments. + +2. **New** `skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md` + (HIGH): + - `/api/artifact/generate/?branch=` is + fire-and-forget. The HTTP 200 means "regen request + accepted", not "regen finished". + - The async regen can run before a recent generator's + group-membership write is visible on the read replica + → race window where the regen sees the wrong group state. + - Required pattern: + - POST `/api/artifact/generate/...` + - Poll `CoreArtifact` filtered by the artifact definition + and target group + - Wait until count matches expected OR re-POST once on + miss + - Hard timeout with a warning so the user sees the gap + - Code skeleton showing a polling loop with `client.filters`, + `asyncio.sleep`, and a timeout. + - When this matters: any caller that triggers regen + programmatically (catalog page, CI job, generator + orchestration). Manual regen in the UI doesn't need this. + +### Section index + +3. **Edit** `skills/infrahub-managing-transforms/rules/_sections.md` + to add the new `queries-` prefix and extend the `artifacts-` + entry to mention async-regen polling. + +### Grader infrastructure + +4. **New** `graders/managing-transforms/__init__.py` (empty + package marker). + +5. **New** `graders/managing-transforms/lib.py`: + - I/O: `load_output_gql(path)` (returns raw text — full + GraphQL parsing not required) and `load_output_py(path)` + (returns parsed AST tree). + - `find_inline_fragments(gql_text) -> list[str]` — return + all `... on ` matches. + - `field_appears_directly_under(gql_text, relationship, + field) -> bool` — heuristic: does the query select + `` inside a ` { node { … } + }` block *without* a preceding `... on` fragment? Used + to flag the bug 1 pattern. + - `has_post_to_artifact_generate(tree) -> bool` — AST scan + for `httpx.post`, `requests.post`, or `client.post` calls + whose URL string literal contains `/api/artifact/generate`. + - `has_loop_construct(tree) -> bool` — at least one + `ast.While` or `ast.For` exists. + - `references_core_artifact_after_post(tree) -> bool` — + after the regen POST, the code includes a + `client.filters(kind="CoreArtifact", …)` or + `client.get(kind="CoreArtifact", …)` call. + - `CHECKS` registry; `run_checks(check_names, output_paths)` + entry point. + +6. **New** `graders/managing-transforms/check_query_union_fragments.py` + — task grader for the union-fragments eval. + +7. **New** `graders/managing-transforms/check_artifact_regen_polling.py` + — task grader for the artifact-polling eval. + +### Eval tasks (added to root `eval.yaml`) + +8. **Task** `transform-query-union-fragments` + (rule #1, deterministic grader): + - Prompt: "Write a GraphQL query for a Jinja2 transform that + fetches DcimDevice with the device's name, role, and its + location's name + shortname. Note that location is a union + of LocationSite | LocationBuilding | LocationHosting — + LocationHosting has no name field, so handle it + explicitly." + - Output: `output.gql`. + - Assertions: + - `query-uses-inline-fragments-for-location` + - `query-no-direct-field-on-union-location` + +9. **Task** `transform-artifact-regen-polling` + (rule #2, deterministic grader): + - Prompt: "Write a Python helper that triggers regeneration + for an artifact definition (POST to + `/api/artifact/generate/?branch=`) and + waits for completion. Don't return success until the + expected number of CoreArtifact instances exist for that + definition + target group. Time out after 60 seconds with + a warning." + - Output: `output.py`. + - Assertions: + - `posts-artifact-generate-endpoint` + - `has-polling-loop` + - `polls-coreartifact-after-post` + +### Sync + docs + +10. Run `python scripts/sync-evals.py` to regenerate + `evaluations/infrahub-managing-transforms.json` (new file, + first evals for this skill). + +11. Update `CHANGELOG.md` `[Unreleased]` section with the + transforms additions. + +## Implementation order + +1. Write the two rule files + update `_sections.md`. +2. Scaffold `graders/managing-transforms/` (lib.py skeleton + + `__init__.py`). +3. Add I/O + AST + GraphQL-text helpers to lib.py (TDD). +4. Add check functions for union-fragments (TDD). +5. Add check functions for artifact-polling (TDD). +6. Write the two task grader scripts. +7. Verify each grader locally against compliant + violating + fixtures. +8. Add the two eval tasks to `eval.yaml`. +9. Run `python scripts/sync-evals.py`. +10. Update `CHANGELOG.md`. + +## Risks and mitigations + +- **GraphQL text parsing is fragile.** Using regex/text matches + instead of a real GraphQL parser means edge cases (multi-line + queries, comments) could trick the grader. Mitigation: the + check only fires on simple direct-field patterns; an + unparseable but valid query falls through to "passes." +- **Artifact polling heuristic is loose.** The three-piece + check (POST + loop + CoreArtifact reference) can be satisfied + by code that doesn't actually poll correctly (e.g., the loop + is for an unrelated purpose). Mitigation: this is acceptable + for an eval bar — the goal is to catch "the model didn't + poll at all," not to verify polling semantics. Document the + limitation in lib.py. +- **No GraphQL union list in schema introspection at eval + time.** The grader can't verify which relationships are + *actually* union-typed in the user's schema. It only checks + for explicit `... on` syntax when `location { node { ... } }` + appears. Mitigation: scope the eval to a known union + relationship (`location`) and document the assumption. + +## Out of scope + +- PR 1 (already shipped as `general-improvements` 1-14 + commits): managing-generators rules. +- PR 3: cross-cutting GraphQL dry-run validation rule in + `infrahub-common`. +- PR 4: hook activation wording. +- Open issues #22-29 (menu and generator concerns) — none + addressed here. + +## Acceptance + +PR 2 is done when: + +- Both new rule files committed under + `skills/infrahub-managing-transforms/rules/`. +- `_sections.md` updated to register `queries-` prefix. +- `graders/managing-transforms/` exists with `lib.py` + 2 + task graders + `__init__.py`. +- `tests/graders/test_transforms_lib.py` exists with unit + tests covering all new helpers + check functions. +- 2 new tasks added to `eval.yaml` with `trials: 3`. +- `evaluations/infrahub-managing-transforms.json` regenerated + and committed. +- Each grader returns correct result on hand-crafted compliant + - violating fixtures. +- Full test suite passes (existing tests not broken). +- `CHANGELOG.md` updated. +- (Skip live `skillgrade --smoke` for the same reason as PR 1 + — skillgrade CLI not installed locally.) diff --git a/eval.yaml b/eval.yaml index e16a12f..818a098 100644 --- a/eval.yaml +++ b/eval.yaml @@ -666,3 +666,281 @@ tasks: check: >- Targeted check_definitions entry declares both targets and parameters + + # --- Generators: relationship reference encoding --- # + - name: generator-relationship-hfid-encoding + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a DcimDevice named 'cEdge-1' + with these single-component HFID references on the schema: + - device_type: 'cEdge-1000' (DcimDeviceType.human_friendly_id = [name__value]) + - manufacturer: 'Cisco' (OrganizationManufacturer.human_friendly_id = [name__value]) + - platform: 'ios-xe' (DcimPlatform.human_friendly_id = [name__value]) + + The generator must use the documented HFID dict form for each relationship, + not bare strings, and the HFID lists must be single-element (matching the + schema's human_friendly_id length). + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_relationship_hfid_encoding.py + weight: 1.0 + expected_output: >- + A Python generator class that calls self.client.create with each + relationship field set to {"hfid": [""]} — a single-element + list inside a dict with the literal key "hfid". No bare strings. + expectations: + - >- + Generator inherits from InfrahubGenerator and implements async generate() + - >- + Each relationship field uses the {"hfid": [""]} dict form + - >- + No relationship field is a bare string (would be treated as id) + - >- + HFID lists for single-component targets are length 1 (no over-pack) + - "save() is called with allow_upsert=True" + assertions: + - name: relationship-hfid-form-correct + check: >- + Every relationship key in client.create(data=...) is either a + {"hfid": [...]} dict, {"id": "..."} dict, or a variable reference. + - name: no-bare-string-relationship + check: >- + No relationship key has a bare string literal as its value. + - name: no-overpacked-hfid-list + check: >- + Single-component HFID targets (device_type, manufacturer, platform) + receive an HFID list of length 1. + + - name: generator-relationship-three-forms + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a DcimDevice 'cEdge-1' and + sets its relationships using all three forms documented in the skill: + + - manufacturer: by HFID (the name 'Cisco' — DcimDevice.manufacturer is a + single-component HFID). + - device_type: by explicit ID, where the UUID is obtained from a prior + GraphQL query and is available in scope as a string variable. + - site: by SDK object reference, where you first call self.client.get + for the LocationSite and pass the returned object directly. + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_relationship_three_forms.py + weight: 1.0 + expected_output: >- + A Python generator that exercises each of the three relationship + reference forms (HFID dict, ID dict, SDK object reference) on the + same client.create call. + expectations: + - "At least one relationship uses {'hfid': ['']}" + - "At least one relationship uses {'id': }" + - >- + At least one relationship passes a variable (SDK object) from + a prior self.client.get call + assertions: + - name: hfid-form-for-name-lookup + check: "At least one relationship uses {'hfid': [...]} form" + - name: id-form-for-uuid + check: "At least one relationship uses {'id': ...} form" + - name: sdk-object-reference-used + check: At least one relationship is a variable reference + + - name: generator-multi-peer-iteration + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates a CoreStandardGroup named + 'sdwan-edges' and adds five DcimDevice instances ('edge-1' through + 'edge-5') as members of that group via group.members.add(). + + Save ONLY the final Python generator class to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_multi_peer_iteration.py + weight: 1.0 + expected_output: >- + A Python generator that adds each peer to the group with one .add() + call per peer (typically inside a for loop), then a single .save(). + No list is passed to .add() in a single call. + expectations: + - "Group is created or fetched via self.client.get / self.client.create" + - ".add() is called once per peer (inside a for loop or 5 explicit calls)" + - "No call passes a list of peers as a single .add() argument" + assertions: + - name: no-list-passed-to-add + check: No .add() call receives a list as its argument + - name: members-add-iterates + check: .add() is invoked inside a for loop or multiple times explicitly + + - name: generator-natural-key-preflight + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write a Python script that creates an IpamPrefix '10.250.10.0/24' + from user input. The prefix may already have been seeded by an earlier + bootstrap run. Handle the collision gracefully — either pre-flight check + for an existing prefix (and exit with a clear message) or save with + allow_upsert=True. Do not let a raw server-side uniqueness error escape. + + Save ONLY the final Python script to: output.py + graders: + - type: deterministic + run: python graders/managing-generators/check_natural_key_preflight.py + weight: 1.0 + expected_output: >- + A Python script that either uses client.get with the natural key + before calling client.create, or passes allow_upsert=True to save(). + expectations: + - "Calls client.create(kind='IpamPrefix', ...)" + - >- + Either calls client.get(kind='IpamPrefix', ...) before create, + or passes allow_upsert=True to save() + - "No bare client.create + save() without one of the two patterns" + assertions: + - name: preflight-or-upsert + check: >- + Either client.get(kind='IpamPrefix', ...) precedes create, OR + save(allow_upsert=True) is called. + - name: no-raw-create-without-handler + check: >- + client.create has either a preflight get OR allow_upsert=True. + + - name: generator-process-integration-test + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-generators/SKILL.md and + follow its workflow and rules. + + Task: Write an Infrahub Generator that creates three DcimDevice instances + from a TopologyDataCenter design. After the generator class, include a + short section (as a Python triple-quoted string named TEST_PLAN at the + bottom of the file, or as a # block comment) explaining how to test the + generator end-to-end against a live Infrahub instance before merging. + + Save ONLY the final Python file to: output.py + graders: + - type: llm_rubric + provider: anthropic + weight: 1.0 + rubric: | + The model output is a Python file at output.py. Score each + criterion 0.0 or 1.0; return the average. + + 1. A generator class is present and its generate() method + creates DcimDevice instances (via self.client.create or + similar). + 2. The file contains either a Python triple-quoted string + named TEST_PLAN, or a block of # comments, that explains + how to test the generator end-to-end. Mentions + "infrahubctl generator run" (or equivalent live-instance + command). At least one full sentence of guidance — not + just a stub. + 3. The guidance explicitly notes that unit tests on the + input dict do not cover SDK call shape (or equivalent + warning that runtime SDK shape errors slip past + type/unit checks). + expected_output: >- + A Python generator plus end-to-end test guidance. The guidance should + mention infrahubctl generator run against a live instance and note + that input-dict unit tests don't cover SDK call shape. + expectations: + - "Generator class is present and creates DcimDevice instances" + - "TEST_PLAN string or block comment exists at the bottom of the file" + - >- + The guidance mentions infrahubctl generator run against a real + Infrahub instance + - >- + The guidance notes that unit tests on the input dict do not cover + SDK call shape + + # --- Transforms: union-typed relationship queries --- # + - name: transform-query-union-fragments + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-transforms/SKILL.md and + follow its workflow and rules. + + Task: Write a GraphQL query for a Jinja2 transform that fetches DcimDevice + with the device's name, its role, and its location's name + shortname. + location is a union of LocationSite | LocationBuilding | LocationHosting + in this repo's base schema. LocationHosting has neither name nor shortname, + so the query must handle the union explicitly so that schema-sync doesn't + fail when a LocationHosting instance is present. + + Save ONLY the final GraphQL query to: output.gql + graders: + - type: deterministic + run: python graders/managing-transforms/check_query_union_fragments.py + weight: 1.0 + expected_output: >- + A GraphQL query that uses inline fragments (... on LocationSite { ... } + and similar) for the location relationship, so fields that exist on + some concrete types but not others are scoped to the correct type. + expectations: + - "Query touches DcimDevice and location" + - "Inline fragments (... on Location...) are used inside location.node" + - "name and shortname are not selected directly under location.node" + assertions: + - name: query-uses-inline-fragments-for-location + check: >- + location { node { ... } } contains at least one ... on Location + inline fragment + - name: query-no-direct-field-on-union-location + check: >- + Neither 'name' nor 'shortname' appears as a direct selection under + location.node without an enclosing inline fragment + + - name: transform-artifact-regen-polling + trials: 3 + instruction: | + Read the skill at .agents/skills/infrahub-managing-transforms/SKILL.md and + follow its workflow and rules. + + Task: Write a Python async helper named regenerate_and_wait that triggers + regeneration for an Infrahub artifact definition by POSTing to + /api/artifact/generate/?branch=, then waits for completion + by polling CoreArtifact filtered by the definition until at least + `expected_count` instances exist. Time out after 60 seconds and raise + TimeoutError. The helper takes (client, def_id, expected_count, branch) + as parameters. + + Save ONLY the final Python helper to: output.py + graders: + - type: deterministic + run: python graders/managing-transforms/check_artifact_regen_polling.py + weight: 1.0 + expected_output: >- + A Python async function that POSTs to /api/artifact/generate/, + then polls CoreArtifact in a loop until count >= expected_count, with + a hard timeout. + expectations: + - "Helper POSTs to /api/artifact/generate/?branch=" + - "Helper contains a loop (while or for) that waits for completion" + - "Helper polls CoreArtifact via client.filters or client.get with kind='CoreArtifact'" + - "Hard timeout exists (raises after some seconds)" + assertions: + - name: posts-artifact-generate-endpoint + check: >- + The code calls .post(...) with a URL containing + /api/artifact/generate + - name: has-polling-loop + check: >- + A while/for/async for loop is present (not fire-and-forget) + - name: polls-coreartifact-after-post + check: >- + A call passes kind="CoreArtifact" (read of artifact state + after the POST) diff --git a/evaluations/infrahub-managing-generators.json b/evaluations/infrahub-managing-generators.json new file mode 100644 index 0000000..ace03c7 --- /dev/null +++ b/evaluations/infrahub-managing-generators.json @@ -0,0 +1,112 @@ +{ + "skill_name": "infrahub-managing-generators", + "evals": [ + { + "id": 1, + "prompt": "Write an Infrahub Generator that creates a DcimDevice named 'cEdge-1'\nwith these single-component HFID references on the schema:\n- device_type: 'cEdge-1000' (DcimDeviceType.human_friendly_id = [name__value])\n- manufacturer: 'Cisco' (OrganizationManufacturer.human_friendly_id = [name__value])\n- platform: 'ios-xe' (DcimPlatform.human_friendly_id = [name__value])\n\nThe generator must use the documented HFID dict form for each relationship,\nnot bare strings, and the HFID lists must be single-element (matching the\nschema's human_friendly_id length).", + "expected_output": "A Python generator class that calls self.client.create with each relationship field set to {\"hfid\": [\"\"]} \u2014 a single-element list inside a dict with the literal key \"hfid\". No bare strings.", + "files": [], + "expectations": [ + "Generator inherits from InfrahubGenerator and implements async generate()", + "Each relationship field uses the {\"hfid\": [\"\"]} dict form", + "No relationship field is a bare string (would be treated as id)", + "HFID lists for single-component targets are length 1 (no over-pack)", + "save() is called with allow_upsert=True" + ], + "assertions": [ + { + "name": "relationship-hfid-form-correct", + "check": "Every relationship key in client.create(data=...) is either a {\"hfid\": [...]} dict, {\"id\": \"...\"} dict, or a variable reference." + }, + { + "name": "no-bare-string-relationship", + "check": "No relationship key has a bare string literal as its value." + }, + { + "name": "no-overpacked-hfid-list", + "check": "Single-component HFID targets (device_type, manufacturer, platform) receive an HFID list of length 1." + } + ] + }, + { + "id": 2, + "prompt": "Write an Infrahub Generator that creates a DcimDevice 'cEdge-1' and\nsets its relationships using all three forms documented in the skill:\n\n- manufacturer: by HFID (the name 'Cisco' \u2014 DcimDevice.manufacturer is a\n single-component HFID).\n- device_type: by explicit ID, where the UUID is obtained from a prior\n GraphQL query and is available in scope as a string variable.\n- site: by SDK object reference, where you first call self.client.get\n for the LocationSite and pass the returned object directly.", + "expected_output": "A Python generator that exercises each of the three relationship reference forms (HFID dict, ID dict, SDK object reference) on the same client.create call.", + "files": [], + "expectations": [ + "At least one relationship uses {'hfid': ['']}", + "At least one relationship uses {'id': }", + "At least one relationship passes a variable (SDK object) from a prior self.client.get call" + ], + "assertions": [ + { + "name": "hfid-form-for-name-lookup", + "check": "At least one relationship uses {'hfid': [...]} form" + }, + { + "name": "id-form-for-uuid", + "check": "At least one relationship uses {'id': ...} form" + }, + { + "name": "sdk-object-reference-used", + "check": "At least one relationship is a variable reference" + } + ] + }, + { + "id": 3, + "prompt": "Write an Infrahub Generator that creates a CoreStandardGroup named\n'sdwan-edges' and adds five DcimDevice instances ('edge-1' through\n'edge-5') as members of that group via group.members.add().", + "expected_output": "A Python generator that adds each peer to the group with one .add() call per peer (typically inside a for loop), then a single .save(). No list is passed to .add() in a single call.", + "files": [], + "expectations": [ + "Group is created or fetched via self.client.get / self.client.create", + ".add() is called once per peer (inside a for loop or 5 explicit calls)", + "No call passes a list of peers as a single .add() argument" + ], + "assertions": [ + { + "name": "no-list-passed-to-add", + "check": "No .add() call receives a list as its argument" + }, + { + "name": "members-add-iterates", + "check": ".add() is invoked inside a for loop or multiple times explicitly" + } + ] + }, + { + "id": 4, + "prompt": "Write a Python script that creates an IpamPrefix '10.250.10.0/24'\nfrom user input. The prefix may already have been seeded by an earlier\nbootstrap run. Handle the collision gracefully \u2014 either pre-flight check\nfor an existing prefix (and exit with a clear message) or save with\nallow_upsert=True. Do not let a raw server-side uniqueness error escape.", + "expected_output": "A Python script that either uses client.get with the natural key before calling client.create, or passes allow_upsert=True to save().", + "files": [], + "expectations": [ + "Calls client.create(kind='IpamPrefix', ...)", + "Either calls client.get(kind='IpamPrefix', ...) before create, or passes allow_upsert=True to save()", + "No bare client.create + save() without one of the two patterns" + ], + "assertions": [ + { + "name": "preflight-or-upsert", + "check": "Either client.get(kind='IpamPrefix', ...) precedes create, OR save(allow_upsert=True) is called." + }, + { + "name": "no-raw-create-without-handler", + "check": "client.create has either a preflight get OR allow_upsert=True." + } + ] + }, + { + "id": 5, + "prompt": "Write an Infrahub Generator that creates three DcimDevice instances\nfrom a TopologyDataCenter design. After the generator class, include a\nshort section (as a Python triple-quoted string named TEST_PLAN at the\nbottom of the file, or as a # block comment) explaining how to test the\ngenerator end-to-end against a live Infrahub instance before merging.", + "expected_output": "A Python generator plus end-to-end test guidance. The guidance should mention infrahubctl generator run against a live instance and note that input-dict unit tests don't cover SDK call shape.", + "files": [], + "expectations": [ + "Generator class is present and creates DcimDevice instances", + "TEST_PLAN string or block comment exists at the bottom of the file", + "The guidance mentions infrahubctl generator run against a real Infrahub instance", + "The guidance notes that unit tests on the input dict do not cover SDK call shape" + ], + "assertions": [] + } + ] +} diff --git a/evaluations/infrahub-managing-transforms.json b/evaluations/infrahub-managing-transforms.json new file mode 100644 index 0000000..4628541 --- /dev/null +++ b/evaluations/infrahub-managing-transforms.json @@ -0,0 +1,52 @@ +{ + "skill_name": "infrahub-managing-transforms", + "evals": [ + { + "id": 1, + "prompt": "Write a GraphQL query for a Jinja2 transform that fetches DcimDevice\nwith the device's name, its role, and its location's name + shortname.\nlocation is a union of LocationSite | LocationBuilding | LocationHosting\nin this repo's base schema. LocationHosting has neither name nor shortname,\nso the query must handle the union explicitly so that schema-sync doesn't\nfail when a LocationHosting instance is present.", + "expected_output": "A GraphQL query that uses inline fragments (... on LocationSite { ... } and similar) for the location relationship, so fields that exist on some concrete types but not others are scoped to the correct type.", + "files": [], + "expectations": [ + "Query touches DcimDevice and location", + "Inline fragments (... on Location...) are used inside location.node", + "name and shortname are not selected directly under location.node" + ], + "assertions": [ + { + "name": "query-uses-inline-fragments-for-location", + "check": "location { node { ... } } contains at least one ... on Location inline fragment" + }, + { + "name": "query-no-direct-field-on-union-location", + "check": "Neither 'name' nor 'shortname' appears as a direct selection under location.node without an enclosing inline fragment" + } + ] + }, + { + "id": 2, + "prompt": "Write a Python async helper named regenerate_and_wait that triggers\nregeneration for an Infrahub artifact definition by POSTing to\n/api/artifact/generate/?branch=, then waits for completion\nby polling CoreArtifact filtered by the definition until at least\n`expected_count` instances exist. Time out after 60 seconds and raise\nTimeoutError. The helper takes (client, def_id, expected_count, branch)\nas parameters.", + "expected_output": "A Python async function that POSTs to /api/artifact/generate/, then polls CoreArtifact in a loop until count >= expected_count, with a hard timeout.", + "files": [], + "expectations": [ + "Helper POSTs to /api/artifact/generate/?branch=", + "Helper contains a loop (while or for) that waits for completion", + "Helper polls CoreArtifact via client.filters or client.get with kind='CoreArtifact'", + "Hard timeout exists (raises after some seconds)" + ], + "assertions": [ + { + "name": "posts-artifact-generate-endpoint", + "check": "The code calls .post(...) with a URL containing /api/artifact/generate" + }, + { + "name": "has-polling-loop", + "check": "A while/for/async for loop is present (not fire-and-forget)" + }, + { + "name": "polls-coreartifact-after-post", + "check": "A call passes kind=\"CoreArtifact\" (read of artifact state after the POST)" + } + ] + } + ] +} diff --git a/graders/managing-generators/__init__.py b/graders/managing-generators/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/graders/managing-generators/check_multi_peer_iteration.py b/graders/managing-generators/check_multi_peer_iteration.py new file mode 100644 index 0000000..873907c --- /dev/null +++ b/graders/managing-generators/check_multi_peer_iteration.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +"""Grader for the generator-multi-peer-iteration eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "no-list-passed-to-add", + "members-add-iterates", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) diff --git a/graders/managing-generators/check_natural_key_preflight.py b/graders/managing-generators/check_natural_key_preflight.py new file mode 100644 index 0000000..ad0369f --- /dev/null +++ b/graders/managing-generators/check_natural_key_preflight.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +"""Grader for the generator-natural-key-preflight eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "preflight-or-upsert", + "no-raw-create-without-handler", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) diff --git a/graders/managing-generators/check_relationship_hfid_encoding.py b/graders/managing-generators/check_relationship_hfid_encoding.py new file mode 100644 index 0000000..51cf06c --- /dev/null +++ b/graders/managing-generators/check_relationship_hfid_encoding.py @@ -0,0 +1,24 @@ +#!/usr/bin/env python3 +"""Grader for the generator-relationship-hfid-encoding eval. + +Reads ``output.py`` from the CWD and prints skillgrade JSON to stdout. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "relationship-hfid-form-correct", + "no-bare-string-relationship", + "no-overpacked-hfid-list", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) diff --git a/graders/managing-generators/check_relationship_three_forms.py b/graders/managing-generators/check_relationship_three_forms.py new file mode 100644 index 0000000..9d12217 --- /dev/null +++ b/graders/managing-generators/check_relationship_three_forms.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +"""Grader for the generator-relationship-three-forms eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "hfid-form-for-name-lookup", + "id-form-for-uuid", + "sdk-object-reference-used", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, Path("output.py")))) diff --git a/graders/managing-generators/lib.py b/graders/managing-generators/lib.py new file mode 100644 index 0000000..7c2151c --- /dev/null +++ b/graders/managing-generators/lib.py @@ -0,0 +1,571 @@ +"""Shared grader library for infrahub-managing-generators evaluations. + +Provides Python AST parsing helpers, individual assertion check +functions, a CHECKS registry, and the top-level ``run_checks`` +function that returns skillgrade JSON format. + +Graders for this skill parse Python source (the generator class +the model produced as ``output.py``) rather than YAML. Some +expressions cannot be resolved from AST alone — variable +references, function-call results — and the check functions +treat those as "indeterminate" (passing) rather than failing. +The checks fail only on shapes that are demonstrably wrong: +bare string literals where a dict reference is required, +over-packed list literals, list literals passed to ``.add()``. + +Usage (in a per-task grader script):: + + from pathlib import Path + from lib import run_checks + + result = run_checks( + ["relationship-hfid-form-correct", "no-bare-string-relationship"], + Path("output.py"), + ) + print(result) +""" + +from __future__ import annotations + +import ast +from pathlib import Path +from typing import Any, Iterator + + +# --------------------------------------------------------------------------- +# I/O helpers +# --------------------------------------------------------------------------- + + +def load_output_py(path: Path) -> tuple[ast.Module | None, str]: + """Load a Python source file and return ``(parsed_tree, raw_text)``. + + Returns ``(None, "")`` if the file does not exist. + Returns ``(None, raw)`` if the file exists but has a syntax error. + """ + try: + raw = Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return None, "" + try: + tree = ast.parse(raw) + except SyntaxError: + return None, raw + return tree, raw + + +# --------------------------------------------------------------------------- +# AST helpers +# --------------------------------------------------------------------------- + + +def _iter_calls(tree: ast.Module) -> Iterator[ast.Call]: + """Yield every ``ast.Call`` node anywhere in the tree.""" + for node in ast.walk(tree): + if isinstance(node, ast.Call): + yield node + + +def _is_self_client_method(call: ast.Call, method: str) -> bool: + """True if ``call.func`` is ``self.client.``.""" + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != method: + return False + if not isinstance(func.value, ast.Attribute) or func.value.attr != "client": + return False + if not isinstance(func.value.value, ast.Name) or func.value.value.id != "self": + return False + return True + + +def find_client_create_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``self.client.create(...)`` call sites.""" + if tree is None: + return [] + return [c for c in _iter_calls(tree) if _is_self_client_method(c, "create")] + + +def find_client_get_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``self.client.get(...)`` call sites.""" + if tree is None: + return [] + return [c for c in _iter_calls(tree) if _is_self_client_method(c, "get")] + + +def find_relationship_add_calls(tree: ast.Module) -> list[ast.Call]: + """Return all ``.add(...)`` call sites. + + Does not filter by attribute path (``.members``, ``.peers``, etc.) — + the check functions decide whether to narrow further. + """ + if tree is None: + return [] + return [ + c for c in _iter_calls(tree) + if isinstance(c.func, ast.Attribute) and c.func.attr == "add" + ] + + +def get_kwarg(call: ast.Call, name: str) -> ast.AST | None: + """Return the value node for keyword argument ``name``, or None.""" + for kw in call.keywords: + if kw.arg == name: + return kw.value + return None + + +def get_data_dict_items(call: ast.Call) -> dict[str, ast.AST]: + """If ``data=`` is a dict literal, return ``{key_str: value_ast}``. + + Returns ``{}`` if ``data`` is missing or not a literal dict. + Non-string keys in the dict literal are skipped silently. + """ + data = get_kwarg(call, "data") + if not isinstance(data, ast.Dict): + return {} + items: dict[str, ast.AST] = {} + for k, v in zip(data.keys, data.values): + if isinstance(k, ast.Constant) and isinstance(k.value, str): + items[k.value] = v + return items + + +def is_hfid_dict(node: ast.AST) -> tuple[bool, list[ast.AST] | None]: + """True if ``node`` is ``{"hfid": [...]}`` with a list value. + + Returns ``(True, [element_ast_nodes])`` on match, else + ``(False, None)``. + """ + if not isinstance(node, ast.Dict): + return False, None + for k, v in zip(node.keys, node.values): + if isinstance(k, ast.Constant) and k.value == "hfid": + if isinstance(v, ast.List): + return True, list(v.elts) + return False, None + return False, None + + +def is_id_dict(node: ast.AST) -> bool: + """True if ``node`` is ``{"id": }``.""" + if not isinstance(node, ast.Dict): + return False + for k in node.keys: + if isinstance(k, ast.Constant) and k.value == "id": + return True + return False + + +def is_bare_string(node: ast.AST) -> bool: + """True if ``node`` is a bare string constant ``ast.Constant(value=str)``.""" + return isinstance(node, ast.Constant) and isinstance(node.value, str) + + +def is_name_or_attribute(node: ast.AST) -> bool: + """True if ``node`` is a Name or Attribute reference (likely an SDK obj).""" + return isinstance(node, (ast.Name, ast.Attribute)) + + +# --------------------------------------------------------------------------- +# Relationship field checks +# --------------------------------------------------------------------------- + +# Relationship-like attribute names commonly used in this repo's base +# schema. The check inspects only these keys in the ``data=`` dict. +# Attribute-style scalar fields (name, status, description, etc.) are +# intentionally excluded so a bare ``"name": "x"`` doesn't fail. +# +# Maintenance note: this set is an allowlist of known base-schema +# relationship names. Generators referencing relationships outside this +# set (e.g. custom schema additions) will silently pass these checks +# even if shape is wrong. Extend this set when new relationship names +# enter the corpus, or invert to detect-by-value-shape if false +# negatives become a problem. +_RELATIONSHIP_KEYS = { + "device_type", "manufacturer", "platform", "site", "location", + "device", "interface", "connected_to", "endpoint_a", "endpoint_z", + "role", "tenant", "provider", "circuit", "asn", "vlan", "vrf", + "ip_namespace", "address", "prefix", "group", +} + + +def _is_known_relationship_key(key: str) -> bool: + return key in _RELATIONSHIP_KEYS + + +def check_relationship_hfid_form_correct( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """For each relationship field in client.create(data=...), the value is + one of: HFID dict, ID dict, or a Name/Attribute reference. + + Bare strings, list literals (non-HFID), or numeric literals fail. + """ + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + problems: list[str] = [] + for call in creates: + data_items = get_data_dict_items(call) + for key, value in data_items.items(): + if not _is_known_relationship_key(key): + continue + hfid_ok, _elts = is_hfid_dict(value) + if hfid_ok or is_id_dict(value) or is_name_or_attribute(value): + continue + problems.append(f"{key}: {ast.dump(value)[:60]}") + + if problems: + return False, f"Wrong relationship reference shape: {'; '.join(problems)}" + return True, "All relationship references use HFID dict, ID dict, or SDK object" + + +def check_no_bare_string_relationship( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Relationship fields must not be bare string literals (bug 3 pattern).""" + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + bad: list[str] = [] + for call in creates: + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_bare_string(value): + bad.append(f"{key}={value.value!r}") + + if bad: + return False, f"Bare-string relationship values (treated as id): {', '.join(bad)}" + return True, "No bare-string relationship values found" + + +def check_no_overpacked_hfid_list( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Single-component HFID targets must receive an HFID list of length 1. + + Heuristic: for the relationships that resolve to single-component HFIDs in + Infrahub's base schema (DcimDeviceType, DcimPlatform, OrganizationManufacturer + referenced via device_type/platform/manufacturer), the HFID list must be + exactly one element.""" + SINGLE_COMPONENT_RELATIONSHIPS = { + "device_type", "manufacturer", "platform", "site", "location", + "role", "tenant", "provider", + } + + if tree is None: + return False, "No Python source to inspect" + + creates = find_client_create_calls(tree) + if not creates: + return False, "No self.client.create(...) calls found" + + bad: list[str] = [] + for call in creates: + for key, value in get_data_dict_items(call).items(): + if key not in SINGLE_COMPONENT_RELATIONSHIPS: + continue + hfid_ok, elts = is_hfid_dict(value) + if hfid_ok and elts is not None and len(elts) != 1: + bad.append(f"{key} got HFID list of length {len(elts)} (expected 1)") + + if bad: + return False, f"Over-packed HFID list: {', '.join(bad)}" + return True, "No over-packed HFID lists for single-component targets" + + +def check_hfid_form_for_name_lookup( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create uses {"hfid": [...]}.""" + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key): + ok, _ = is_hfid_dict(value) + if ok: + return True, f"{key} uses HFID dict form" + return False, "No relationship uses {'hfid': [...]} form" + + +def check_id_form_for_uuid( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create uses {"id": ...}.""" + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_id_dict(value): + return True, f"{key} uses ID dict form" + return False, "No relationship uses {'id': ...} form" + + +def check_sdk_object_reference_used( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """At least one relationship in client.create is a variable reference + (presumably an SDK object from a prior client.get / client.create call). + """ + if tree is None: + return False, "No Python source to inspect" + for call in find_client_create_calls(tree): + for key, value in get_data_dict_items(call).items(): + if _is_known_relationship_key(key) and is_name_or_attribute(value): + return True, f"{key} uses SDK object reference" + return False, "No relationship passes an SDK object reference" + + +# --------------------------------------------------------------------------- +# Multi-peer add checks +# --------------------------------------------------------------------------- + + +def _is_list_referenced(node: ast.AST, tree: ast.Module) -> bool: + """Best-effort: is ``node`` either a list literal or a Name that was + assigned a list literal earlier in the module? + """ + if isinstance(node, ast.List): + return True + if isinstance(node, ast.Name): + target_name = node.id + for assign in ast.walk(tree): + if not isinstance(assign, ast.Assign): + continue + for tgt in assign.targets: + if isinstance(tgt, ast.Name) and tgt.id == target_name: + if isinstance(assign.value, ast.List): + return True + if isinstance(assign.value, ast.ListComp): + return True + # Also check ann-assigns: `devices: list[T] = [...]` + for ann in ast.walk(tree): + if not isinstance(ann, ast.AnnAssign): + continue + tgt = ann.target + if isinstance(tgt, ast.Name) and tgt.id == target_name: + if isinstance(ann.value, (ast.List, ast.ListComp)): + return True + return False + + +def check_no_list_passed_to_add( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """No .add(...) call may receive a list argument as its sole peer.""" + if tree is None: + return False, "No Python source to inspect" + + add_calls = find_relationship_add_calls(tree) + if not add_calls: + return False, "No .add(...) calls found" + + bad: list[str] = [] + for call in add_calls: + if len(call.args) != 1: + continue + arg = call.args[0] + if _is_list_referenced(arg, tree): + bad.append(ast.unparse(call.func) if hasattr(ast, "unparse") else call.func.attr) + + if bad: + return False, f".add() received a list: {', '.join(bad)}" + return True, "No .add() call received a list argument" + + +def check_members_add_iterates( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """``.add(...)`` should appear inside a For loop OR be called multiple times + on the same attribute path. Both indicate per-peer iteration. + """ + if tree is None: + return False, "No Python source to inspect" + + add_calls = find_relationship_add_calls(tree) + if not add_calls: + return False, "No .add(...) calls found" + + # Look for any For loop containing a .add() call + for for_node in ast.walk(tree): + if not isinstance(for_node, ast.For): + continue + for inner in ast.walk(for_node): + if isinstance(inner, ast.Call) and inner in add_calls: + return True, ".add() is called inside a for loop" + + # If multiple .add() calls share the same attribute path, treat as + # explicit per-peer adds + paths = [] + for call in add_calls: + try: + paths.append(ast.unparse(call.func)) + except Exception: + pass + if len(paths) >= 2 and len(set(paths)) < len(paths): + return True, f".add() called multiple times: {len(add_calls)} calls" + + return False, "Only a single .add() call and not in a for loop" + + +# --------------------------------------------------------------------------- +# Natural-key preflight +# --------------------------------------------------------------------------- + + +def _has_save_with_upsert_true(tree: ast.Module) -> bool: + """True if any ``.save(allow_upsert=True)`` call exists.""" + for call in _iter_calls(tree): + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != "save": + continue + for kw in call.keywords: + if kw.arg == "allow_upsert": + if isinstance(kw.value, ast.Constant) and kw.value.value is True: + return True + return False + + +def _has_client_get_for_same_kind_as_create(tree: ast.Module) -> bool: + """True if there is at least one ``client.get(kind=X)`` AND a + ``client.create(kind=X)`` for the same kind value. + """ + def _kind_arg(call: ast.Call) -> str | None: + for kw in call.keywords: + if kw.arg == "kind" and isinstance(kw.value, ast.Constant): + return kw.value.value + return None + + creates = find_client_create_calls(tree) + gets = find_client_get_calls(tree) + + # Also accept generic ``client.get`` (not self.client.get) for non-generator scripts + for call in _iter_calls(tree): + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != "get": + continue + if isinstance(func.value, ast.Name) and func.value.id == "client": + gets.append(call) + + # Also accept generic ``client.create`` for non-generator scripts + for call in _iter_calls(tree): + func = call.func + if not isinstance(func, ast.Attribute) or func.attr != "create": + continue + if isinstance(func.value, ast.Name) and func.value.id == "client": + creates.append(call) + + create_kinds = {_kind_arg(c) for c in creates if _kind_arg(c)} + get_kinds = {_kind_arg(c) for c in gets if _kind_arg(c)} + return bool(create_kinds & get_kinds) + + +def check_preflight_or_upsert( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """Either a same-kind client.get precedes create, OR save uses upsert.""" + if tree is None: + return False, "No Python source to inspect" + + if _has_save_with_upsert_true(tree): + return True, "save(allow_upsert=True) found" + if _has_client_get_for_same_kind_as_create(tree): + return True, "client.get preflights client.create for the same kind" + return False, "Neither preflight client.get nor save(allow_upsert=True) found" + + +def check_no_raw_create_without_handler( + tree: ast.Module | None, **_: Any +) -> tuple[bool, str]: + """If client.create is used but neither preflight nor upsert is present, + this is the bug 5 pattern. + """ + if tree is None: + return False, "No Python source to inspect" + + has_self_create = bool(find_client_create_calls(tree)) + has_bare_client_create = any( + isinstance(c.func, ast.Attribute) and c.func.attr == "create" + and isinstance(c.func.value, ast.Name) and c.func.value.id == "client" + for c in _iter_calls(tree) + ) + + if not has_self_create and not has_bare_client_create: + return False, "No client.create call found" + + if _has_save_with_upsert_true(tree): + return True, "save(allow_upsert=True) covers the collision case" + if _has_client_get_for_same_kind_as_create(tree): + return True, "preflight client.get covers the collision case" + return False, "client.create has no preflight and no allow_upsert=True" + + +# --------------------------------------------------------------------------- +# CHECKS registry +# --------------------------------------------------------------------------- + +CHECKS: dict[str, Any] = { + "relationship-hfid-form-correct": check_relationship_hfid_form_correct, + "no-bare-string-relationship": check_no_bare_string_relationship, + "no-overpacked-hfid-list": check_no_overpacked_hfid_list, + "hfid-form-for-name-lookup": check_hfid_form_for_name_lookup, + "id-form-for-uuid": check_id_form_for_uuid, + "sdk-object-reference-used": check_sdk_object_reference_used, + "no-list-passed-to-add": check_no_list_passed_to_add, + "members-add-iterates": check_members_add_iterates, + "preflight-or-upsert": check_preflight_or_upsert, + "no-raw-create-without-handler": check_no_raw_create_without_handler, +} + + +# --------------------------------------------------------------------------- +# run_checks — top-level entry point for grader scripts +# --------------------------------------------------------------------------- + + +def run_checks( + check_names: list[str], + output_path: Path, +) -> dict: + """Run named checks against a Python source file. + + Returns skillgrade JSON: ``{"score": float, "details": str, + "checks": [...]}``. + + Raises ``KeyError`` if any name in ``check_names`` is not in + ``CHECKS``. + """ + tree, raw = load_output_py(output_path) + + entries: list[dict] = [] + passed_count = 0 + + for name in check_names: + fn = CHECKS[name] # raises KeyError for unknown names + try: + ok, msg = fn(tree, raw_text=raw) + except Exception as exc: # defensive — never let one check crash all + ok, msg = False, f"Error running check: {exc}" + + if ok: + passed_count += 1 + entries.append({"name": name, "passed": ok, "message": msg}) + + total = len(check_names) + score = round(passed_count / total, 4) if total > 0 else 0.0 + + failed_names = [e["name"] for e in entries if not e["passed"]] + if failed_names: + details = f"{passed_count}/{total} checks passed. Failed: {', '.join(failed_names)}" + else: + details = f"All {total} checks passed." + + return {"score": score, "details": details, "checks": entries} diff --git a/graders/managing-transforms/__init__.py b/graders/managing-transforms/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/graders/managing-transforms/check_artifact_regen_polling.py b/graders/managing-transforms/check_artifact_regen_polling.py new file mode 100644 index 0000000..87cd5d2 --- /dev/null +++ b/graders/managing-transforms/check_artifact_regen_polling.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +"""Grader for the transform-artifact-regen-polling eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "posts-artifact-generate-endpoint", + "has-polling-loop", + "polls-coreartifact-after-post", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, {"py": Path("output.py")}))) diff --git a/graders/managing-transforms/check_query_union_fragments.py b/graders/managing-transforms/check_query_union_fragments.py new file mode 100644 index 0000000..a4fd0ff --- /dev/null +++ b/graders/managing-transforms/check_query_union_fragments.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +"""Grader for the transform-query-union-fragments eval.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from lib import run_checks # noqa: E402 + +CHECKS = [ + "query-uses-inline-fragments-for-location", + "query-no-direct-field-on-union-location", +] + +if __name__ == "__main__": + print(json.dumps(run_checks(CHECKS, {"gql": Path("output.gql")}))) diff --git a/graders/managing-transforms/lib.py b/graders/managing-transforms/lib.py new file mode 100644 index 0000000..3b8252b --- /dev/null +++ b/graders/managing-transforms/lib.py @@ -0,0 +1,379 @@ +"""Shared grader library for infrahub-managing-transforms evaluations. + +Provides text-parsing helpers for ``.gql`` files, Python AST +helpers for ``.py`` files, individual check functions, a +``CHECKS`` registry, and the top-level ``run_checks`` entry +point that returns skillgrade JSON. + +Two output kinds are supported: + +- ``output.gql`` — raw GraphQL query text. The union-fragments + checks use simple regex/text matching rather than a full + GraphQL parser; this is fragile by design but cheap and + matches the failure shape we care about. +- ``output.py`` — Python source for the artifact-regen polling + eval. Checks use AST parsing. + +Usage (in a per-task grader script):: + + from pathlib import Path + from lib import run_checks + + result = run_checks( + ["query-uses-inline-fragments-for-location"], + {"gql": Path("output.gql")}, + ) +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path +from typing import Any, Iterator + + +# --------------------------------------------------------------------------- +# I/O helpers +# --------------------------------------------------------------------------- + + +def load_output_gql(path: Path) -> str: + """Load a GraphQL query file. Returns empty string on missing file.""" + try: + return Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return "" + + +def load_output_py(path: Path) -> tuple[ast.Module | None, str]: + """Load a Python source file and return ``(parsed_tree, raw_text)``. + + Returns ``(None, "")`` if the file does not exist. + Returns ``(None, raw)`` if the file exists but has a syntax error. + """ + try: + raw = Path(path).read_text(encoding="utf-8") + except (FileNotFoundError, OSError): + return None, "" + try: + tree = ast.parse(raw) + except SyntaxError: + return None, raw + return tree, raw + + +# --------------------------------------------------------------------------- +# GraphQL text helpers +# --------------------------------------------------------------------------- + + +_INLINE_FRAGMENT_RE = re.compile(r"\.\.\.\s*on\s+([A-Za-z_][A-Za-z0-9_]*)") + + +def find_inline_fragments(gql_text: str) -> list[str]: + """Return all type names appearing in ``... on ``.""" + return _INLINE_FRAGMENT_RE.findall(gql_text or "") + + +def _find_balanced_block(text: str, start: int) -> str | None: + """Given an index pointing at ``{``, return the substring up to + the matching ``}`` (inclusive). Returns ``None`` on imbalance. + """ + if start >= len(text) or text[start] != "{": + return None + depth = 0 + for i in range(start, len(text)): + ch = text[i] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return text[start : i + 1] + return None + + +def block_for_relationship(gql_text: str, rel_name: str) -> str | None: + """Return the text of `` { ... }`` (first occurrence).""" + pattern = re.compile(rf"\b{re.escape(rel_name)}\s*\{{") + match = pattern.search(gql_text or "") + if not match: + return None + return _find_balanced_block(gql_text, match.end() - 1) + + +def field_appears_directly_under_union( + gql_text: str, rel_name: str, field: str +) -> bool: + """Heuristic: does the query select ```` inside + `` { node { ... } }`` *without* a preceding + ``... on `` fragment in that same node block? + + Returns ``False`` if the relationship isn't queried at all, + or if the query uses inline fragments around the field. + """ + block = block_for_relationship(gql_text, rel_name) + if block is None: + return False + # Find the inner `node { ... }` block + node_match = re.search(r"\bnode\s*\{", block) + if not node_match: + return False + node_block = _find_balanced_block(block, node_match.end() - 1) + if node_block is None: + return False + # If there are inline fragments inside this node block, treat as safe. + if find_inline_fragments(node_block): + return False + # Otherwise, does the field appear as a direct sub-selection? + field_pattern = re.compile(rf"\b{re.escape(field)}\s*\{{") + return bool(field_pattern.search(node_block)) + + +# --------------------------------------------------------------------------- +# Python AST helpers +# --------------------------------------------------------------------------- + + +def _iter_calls(tree: ast.Module) -> Iterator[ast.Call]: + for node in ast.walk(tree): + if isinstance(node, ast.Call): + yield node + + +_ARTIFACT_GENERATE_PATH = "/api/artifact/generate" + + +def _string_contains(node: ast.AST, needle: str) -> bool: + """True if ``node`` is a string literal or f-string containing ``needle``.""" + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return needle in node.value + if isinstance(node, ast.JoinedStr): + return any( + isinstance(v, ast.Constant) and isinstance(v.value, str) and needle in v.value + for v in node.values + ) + return False + + +def _is_post_like_method(attr_name: str) -> bool: + """True if ``attr_name`` is any plausible HTTP-POST method name. + + Matches ``post`` (httpx/requests/aiohttp public method), + ``_post`` (infrahub_sdk's private helper), and anything else + ending in ``post`` (``do_post``, ``async_post``, etc.). + """ + return attr_name.endswith("post") + + +def has_post_to_artifact_generate( + tree: ast.Module | None, py_raw: str = "" +) -> bool: + """True if the source POSTs to ``/api/artifact/generate``. + + Two detection strategies: + + 1. **Direct AST match** — any call to a method whose name ends in + ``post`` (``post``, ``_post``, etc.) where the URL is a string + literal (or f-string) containing the path. Catches the public + HTTP-client ``.post(url)`` pattern and the SDK's private + ``client._post(url=..., payload=...)`` pattern alike. + + 2. **Fallback text+call match** — if (1) doesn't fire, accept + the case where the source contains both *some* post-like call + AND the path as a string anywhere in the file. Covers + realistic LLM output like + ``endpoint = f"...{def_id}..."; await client._post(endpoint)``. + """ + if tree is None: + return False + has_any_post = False + for call in _iter_calls(tree): + func = call.func + if not (isinstance(func, ast.Attribute) and _is_post_like_method(func.attr)): + continue + has_any_post = True + candidates: list[ast.AST] = list(call.args[:1]) + for kw in call.keywords: + if kw.arg in ("url", "path", "endpoint"): + candidates.append(kw.value) + for c in candidates: + if _string_contains(c, _ARTIFACT_GENERATE_PATH): + return True + if has_any_post and py_raw and _ARTIFACT_GENERATE_PATH in py_raw: + return True + return False + + +def has_loop_construct(tree: ast.Module | None) -> bool: + """True if ``ast.While`` or ``ast.For`` (sync or async) is in tree.""" + if tree is None: + return False + for node in ast.walk(tree): + if isinstance(node, (ast.While, ast.For, ast.AsyncFor)): + return True + return False + + +def references_core_artifact_in_call(tree: ast.Module | None) -> bool: + """True if any call passes ``kind="CoreArtifact"`` as a keyword arg.""" + if tree is None: + return False + for call in _iter_calls(tree): + for kw in call.keywords: + if kw.arg == "kind" and isinstance(kw.value, ast.Constant): + if kw.value.value == "CoreArtifact": + return True + return False + + +# --------------------------------------------------------------------------- +# Union-fragments checks +# --------------------------------------------------------------------------- + +# Known union-typed relationships in Infrahub base schema. The +# grader is intentionally narrow — extend this dict when new +# unions enter the eval corpus. +_KNOWN_UNION_RELATIONSHIPS = { + "location": ("name", "shortname"), # rel name -> divergent fields +} + + +def check_query_uses_inline_fragments_for_location( + gql_text: str = "", **_: Any +) -> tuple[bool, str]: + """If the query touches ``location``, it must contain at least one + ``... on Location<...>`` inline fragment. + """ + if not gql_text: + return False, "No GraphQL output to inspect" + block = block_for_relationship(gql_text, "location") + if block is None: + return False, "Query does not touch 'location' relationship" + fragments = find_inline_fragments(block) + if any(f.startswith("Location") for f in fragments): + return True, f"Uses inline fragments: {fragments}" + return False, "location { ... } contains no ... on Location fragment" + + +def check_query_no_direct_field_on_union_location( + gql_text: str = "", **_: Any +) -> tuple[bool, str]: + """The query must not select ``name``/``shortname`` directly on + the ``location`` union (the bug 1 pattern). + """ + if not gql_text: + return False, "No GraphQL output to inspect" + bad: list[str] = [] + for field in _KNOWN_UNION_RELATIONSHIPS["location"]: + if field_appears_directly_under_union(gql_text, "location", field): + bad.append(field) + if bad: + return False, f"Direct field(s) on union location.node: {', '.join(bad)}" + return True, "No direct field selections on union location.node" + + +# --------------------------------------------------------------------------- +# Artifact regen polling checks +# --------------------------------------------------------------------------- + + +def check_posts_artifact_generate_endpoint( + tree: ast.Module | None = None, py_raw: str = "", **_: Any +) -> tuple[bool, str]: + """Source must contain a POST whose URL mentions /api/artifact/generate.""" + if tree is None: + return False, "No Python source to inspect" + if has_post_to_artifact_generate(tree, py_raw): + return True, "POST to /api/artifact/generate found" + return False, "No POST to /api/artifact/generate found" + + +def check_has_polling_loop( + tree: ast.Module | None = None, **_: Any +) -> tuple[bool, str]: + """Source must contain at least one ``while``/``for``/``async for`` loop.""" + if tree is None: + return False, "No Python source to inspect" + if has_loop_construct(tree): + return True, "Loop construct found" + return False, "No loop construct found — fire-and-forget pattern" + + +def check_polls_coreartifact_after_post( + tree: ast.Module | None = None, py_raw: str = "", **_: Any +) -> tuple[bool, str]: + """Source must reference ``kind="CoreArtifact"`` in a call (a read).""" + if tree is None: + return False, "No Python source to inspect" + if not has_post_to_artifact_generate(tree, py_raw): + return False, "No POST to /api/artifact/generate; nothing to poll" + if references_core_artifact_in_call(tree): + return True, "CoreArtifact read found after POST" + return False, "No call references kind='CoreArtifact'" + + +# --------------------------------------------------------------------------- +# CHECKS registry +# --------------------------------------------------------------------------- + +CHECKS: dict[str, Any] = { + "query-uses-inline-fragments-for-location": check_query_uses_inline_fragments_for_location, + "query-no-direct-field-on-union-location": check_query_no_direct_field_on_union_location, + "posts-artifact-generate-endpoint": check_posts_artifact_generate_endpoint, + "has-polling-loop": check_has_polling_loop, + "polls-coreartifact-after-post": check_polls_coreartifact_after_post, +} + + +# --------------------------------------------------------------------------- +# run_checks — top-level entry point +# --------------------------------------------------------------------------- + + +def run_checks( + check_names: list[str], + output_paths: dict[str, Path], +) -> dict: + """Run named checks against one or more output files. + + Parameters + ---------- + check_names: + List of assertion names from ``CHECKS``. + output_paths: + Mapping of output kind to path. Recognised keys: ``"gql"``, + ``"py"``. Each check function declares which input it + needs via ``**kwargs``. + + Returns skillgrade JSON ``{"score", "details", "checks"}``. + Raises ``KeyError`` if any check name is unknown. + """ + gql_text = load_output_gql(output_paths.get("gql", Path("output.gql"))) + tree, py_raw = load_output_py(output_paths.get("py", Path("output.py"))) + + entries: list[dict] = [] + passed_count = 0 + + for name in check_names: + fn = CHECKS[name] + try: + ok, msg = fn(gql_text=gql_text, tree=tree, py_raw=py_raw) + except Exception as exc: # defensive — never let one check crash all + ok, msg = False, f"Error running check: {exc}" + + if ok: + passed_count += 1 + entries.append({"name": name, "passed": ok, "message": msg}) + + total = len(check_names) + score = round(passed_count / total, 4) if total > 0 else 0.0 + + failed = [e["name"] for e in entries if not e["passed"]] + if failed: + details = f"{passed_count}/{total} checks passed. Failed: {', '.join(failed)}" + else: + details = f"All {total} checks passed." + + return {"score": score, "details": details, "checks": entries} diff --git a/hooks-handlers/session-start.sh b/hooks-handlers/session-start.sh index 1889acc..948c826 100755 --- a/hooks-handlers/session-start.sh +++ b/hooks-handlers/session-start.sh @@ -28,7 +28,7 @@ if [[ "$INFRAHUB_DETECTED" == "true" ]]; then { "hookSpecificOutput": { "hookEventName": "SessionStart", - "additionalContext": "This is an Infrahub project. When the user asks about schema design, data modeling, or infrastructure data management tasks, use the `infrahub-managing-schemas` skill from the infrahub plugin.\n\nThe skill documentation is located at: ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-schemas/\n\nKey files to reference:\n- SKILL.md - Overview and quick start\n- reference.md - Complete schema property reference (nodes, attributes, relationships, generics)\n- validation.md - Schema validation commands and migration guide\n- examples.md - Ready-to-use schema templates\n\nWhen working with Infrahub schemas:\n1. Read the relevant skill documentation files before making schema changes\n2. Follow Infrahub naming conventions (PascalCase for nodes/generics, snake_case for attributes)\n3. Always include human_friendly_id for nodes\n4. Set identifier on bidirectional relationships\n5. Use generics for shared attributes across multiple node types\n6. Validate schemas with `infrahubctl schema check` before loading\n\nAdditional skills available:\n- `infrahub-managing-objects` - Create data objects (devices, locations, etc.) at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-objects/\n- `infrahub-managing-checks` - Create validation checks at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-checks/\n- `infrahub-managing-generators` - Create design-driven generators at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-generators/\n- `infrahub-managing-transforms` - Create data transforms at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-transforms/\n- `infrahub-managing-menus` - Create navigation menus at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-menus/\n- `infrahub-analyzing-data` - Query and correlate live Infrahub data via the MCP server at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-analyzing-data/\n- `infrahub-auditing-repo` - Audit the repository against all rules and best practices at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-auditing-repo/\n- Shared references and rules at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/\n\nBefore running any `infrahubctl` command, detect the correct Python environment: try `uv run infrahubctl info`, then `poetry run infrahubctl info`, then `infrahubctl info` directly — use the first that succeeds as the prefix for all subsequent commands. See ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/rules/connectivity-python-environment.md for the full detection rule and ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/rules/connectivity-server-check.md for server connectivity troubleshooting." + "additionalContext": "This is an Infrahub project. When the user asks about schema design, data modeling, or infrastructure data management tasks, use the `infrahub-managing-schemas` skill from the infrahub plugin.\n\nThe skill documentation is located at: ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-schemas/\n\nKey files to reference:\n- SKILL.md - Overview and quick start\n- reference.md - Complete schema property reference (nodes, attributes, relationships, generics)\n- validation.md - Schema validation commands and migration guide\n- examples.md - Ready-to-use schema templates\n\nWhen working with Infrahub schemas:\n1. Read the relevant skill documentation files before making schema changes\n2. Follow Infrahub naming conventions (PascalCase for nodes/generics, snake_case for attributes)\n3. Always include human_friendly_id for nodes\n4. Set identifier on bidirectional relationships\n5. Use generics for shared attributes across multiple node types\n6. Validate schemas with `infrahubctl schema check` before loading\n\nAdditional skills available:\n- `infrahub-managing-objects` - Create data objects (devices, locations, etc.) at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-objects/\n- `infrahub-managing-checks` - Create validation checks at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-checks/\n- `infrahub-managing-generators` - Create design-driven generators at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-generators/\n- `infrahub-managing-transforms` - Create data transforms at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-transforms/\n- `infrahub-managing-menus` - Create navigation menus at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-managing-menus/\n- `infrahub-analyzing-data` - Query and correlate live Infrahub data via the MCP server at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-analyzing-data/\n- `infrahub-auditing-repo` - Audit the repository against all rules and best practices at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-auditing-repo/\n- Shared references and rules at ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/\n\nSkill activation triggers — read the corresponding skill BEFORE writing or editing code in these paths. The integration-layer rules (relationship encoding, union-typed queries, async artifact regen, multi-peer .add() iteration, natural-key preflight) live in the skills, not in the source code, so working from pattern-matching alone has historically produced bugs that pass unit tests but fail against a live Infrahub server:\n- `generators/**/*.py` or `.gql` → MUST read `infrahub-managing-generators` SKILL.md plus rules/python-relationship-references.md and rules/python-multi-peer-add.md\n- `transforms/**/*.py`, `templates/**/*.j2`, or `queries/**/*.gql` → MUST read `infrahub-managing-transforms` SKILL.md plus rules/queries-union-fragments.md before editing any .gql, and rules/artifacts-async-regen-polling.md before any code that POSTs to /api/artifact/generate\n- `checks/**/*.py` → MUST read `infrahub-managing-checks` SKILL.md\n- `objects/**/*.yml`, `data/**/*.yml`, or any apiVersion: infrahub.app/v1 + kind: Object file → MUST read `infrahub-managing-objects` SKILL.md\n- `menus/**/*.yml` or any kind: Menu file → MUST read `infrahub-managing-menus` SKILL.md\n- Any `.gql` change in any directory → also check `infrahub-common/rules/deployment-gql-dry-run.md` for the pre-merge dry-run validation rule\n\nBefore running any `infrahubctl` command, detect the correct Python environment: try `uv run infrahubctl info`, then `poetry run infrahubctl info`, then `infrahubctl info` directly — use the first that succeeds as the prefix for all subsequent commands. See ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/rules/connectivity-python-environment.md for the full detection rule and ${CLAUDE_PLUGIN_ROOT}/skills/infrahub-common/rules/connectivity-server-check.md for server connectivity troubleshooting." } } EOF diff --git a/skills/infrahub-common/rules/_sections.md b/skills/infrahub-common/rules/_sections.md index ba86aa2..5566701 100644 --- a/skills/infrahub-common/rules/_sections.md +++ b/skills/infrahub-common/rules/_sections.md @@ -8,7 +8,10 @@ specific to any single workflow. 1. **Deployment (deployment-)** -- CRITICAL. Git repository integration, CoreRepository vs CoreReadOnlyRepository, local dev setup, worker race - conditions, file commit requirements. + conditions, file commit requirements, and pre-merge + dry-run validation of `.gql` queries against a live + schema (YAML-check alone misses GraphQL/schema + mismatches). 2. **Protocols (protocols-)** -- CRITICAL. Protocol files are generated code (`infrahubctl protocols generate`), diff --git a/skills/infrahub-common/rules/deployment-gql-dry-run.md b/skills/infrahub-common/rules/deployment-gql-dry-run.md new file mode 100644 index 0000000..edcec42 --- /dev/null +++ b/skills/infrahub-common/rules/deployment-gql-dry-run.md @@ -0,0 +1,83 @@ +--- +title: Dry-Run GraphQL Queries Against the Live Schema Before Merge +impact: HIGH +tags: deployment, gql, validation, dry-run, schema-sync, pre-merge +--- + +## Dry-Run GraphQL Queries Against the Live Schema Before Merge + +**Impact:** HIGH + +YAML schema validation (`infrahubctl schema check`) and Python +type checking catch lots of mistakes, but they **don't catch +GraphQL query/schema mismatches**. A query that asks for a +field that doesn't exist on a type — or asks for a field on +a union type without inline fragments — passes every static +check, and only fails when `CoreRepository` actually executes +the query during schema-sync. + +When a `.gql` file under `queries/**/` is wrong, the typical +failure shape is: + +- `CoreRepository` sync hangs in `error-import` state +- Zero generator / transform / check definitions register +- Downstream pipelines (`invoke init`, proposed-change + validation) time out with no obvious root cause + +This is a *silent* failure from the developer's perspective — +the YAML is fine, the Python is fine, the only signal is that +nothing runs. + +### The rule + +Before opening a PR that touches any `.gql` file under +`queries/**/`, run each affected query against a live +Infrahub schema. + +```bash +# Per-query dry-run via infrahubctl +infrahubctl render --dry-run + +# For a check or generator, the equivalent is to run the +# check/generator itself locally — it will fetch via the .gql +# and surface any GraphQL error on the spot +infrahubctl check run +infrahubctl generator run +``` + +If your local Infrahub doesn't have data matching the query, +spin up a fresh instance with the bootstrap dataset +(`invoke init` or equivalent) so the query exercises real +shapes — empty datasets hide union-fragment bugs because no +concrete instance is returned to fail on. + +### What this catches that YAML-check misses + +| Failure | Caught by YAML check? | Caught by dry-run? | +| ------- | --------------------- | ------------------ | +| `kind:` typo in schema | Yes | Yes | +| Indentation / structure error | Yes | Yes | +| `human_friendly_id` referencing missing attr | Yes | Yes | +| Querying a field that doesn't exist on the target type | No | Yes | +| Querying a field on a union without inline fragments (and the union contains an inheritor that lacks the field) | No | Yes | +| Filter argument typo | No | Yes | + +The two non-caught cases are the most common silent-failure +sources in production schema-sync. + +### When to skip + +Trivial query edits that only adjust whitespace, comments, or +the order of explicitly-selected scalar fields don't need a +dry-run. Any change to a field selection, a filter argument, +a fragment, or a relationship traversal does. + +### CI integration + +Where practical, wire `infrahubctl render --dry-run` into CI +as a pre-merge gate on `queries/**/*.gql` changes. The check +takes <1s per query against a warmed Infrahub and prevents +the silent-sync-failure class of bug from reaching main. + +Reference: +[Infrahub schema docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-generators/rules/_sections.md b/skills/infrahub-managing-generators/rules/_sections.md index c16b401..052d449 100644 --- a/skills/infrahub-managing-generators/rules/_sections.md +++ b/skills/infrahub-managing-generators/rules/_sections.md @@ -8,7 +8,9 @@ 2. **Python Class (python-)** -- CRITICAL. InfrahubGenerator base class, async generate() method, object creation via self.client.create(), save with - allow_upsert=True, relationship references by ID. + allow_upsert=True, relationship references via HFID dict / + ID dict / SDK object (never bare string), and multi-peer + add iteration on RelationshipManager. 3. **Tracking (tracking-)** -- HIGH. Automatic cleanup of stale objects via delete_unused_nodes=True, idempotent @@ -25,7 +27,10 @@ 6. **Patterns (patterns-)** -- MEDIUM. Data cleaning helper, batch object creation, using the local store for - inter-object references. + inter-object references, and natural-key preflight for + form-driven mutations. 7. **Testing (testing-)** -- LOW. infrahubctl generator - commands, listing and running Generators locally. + commands, listing and running Generators locally, and the + integration-test workflow (run end-to-end against a live + instance before declaring done). diff --git a/skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md b/skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md new file mode 100644 index 0000000..353ffc1 --- /dev/null +++ b/skills/infrahub-managing-generators/rules/patterns-natural-key-preflight.md @@ -0,0 +1,79 @@ +--- +title: Pre-flight Natural-Key Check for Form-Driven Mutations +impact: MEDIUM +tags: patterns, idempotent, uniqueness, upsert, form, catalog +--- + +## Pre-flight Natural-Key Check for Form-Driven Mutations + +Impact: MEDIUM + +Inside ``InfrahubGenerator.generate()`` the default ``save()`` +already happens with ``allow_upsert=True`` because the tracking +context wraps the call. **Outside that context** — in scripts, +Streamlit pages, or any ad-hoc code that takes user input and +calls ``client.create`` directly — uniqueness collisions on +bootstrap-seeded keys surface as raw server exceptions in the +middle of a half-built workflow. + +Always pick one of two patterns for catalog-style mutations. + +### Pattern A — pre-flight check + +Use when the desired behavior is "fail loudly with a friendly +message if the object already exists." + +```python +from infrahub_sdk.exceptions import NodeNotFound + +try: + existing = await client.get( + kind="IpamPrefix", + prefix__value=user_input_prefix, + ) + st.error(f"Prefix {user_input_prefix} already exists.") + return +except NodeNotFound: + pass + +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save() +``` + +### Pattern B — upsert + +Use when the desired behavior is "create or update silently." +This is the default inside generators. + +```python +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save(allow_upsert=True) +``` + +### Anti-pattern + +```python +# WRONG — uniqueness collision surfaces as raw server exception +prefix = await client.create( + kind="IpamPrefix", + data={"prefix": user_input_prefix}, +) +await prefix.save() # no upsert, no preflight +``` + +### How to decide + +- **Form-driven UI catalog pages:** Pattern A. Give the user a + legible "already exists" message. +- **Generators with tracking:** ``allow_upsert=True`` is the + norm — the generator should be idempotent. +- **Bootstrap scripts and migrations:** Pattern B (upsert) + unless you have a specific reason to fail on existing data. + +Reference: [Infrahub SDK Docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-generators/rules/python-generate.md b/skills/infrahub-managing-generators/rules/python-generate.md index dd34f6f..f079321 100644 --- a/skills/infrahub-managing-generators/rules/python-generate.md +++ b/skills/infrahub-managing-generators/rules/python-generate.md @@ -38,14 +38,15 @@ class MyGenerator(InfrahubGenerator): ### Object Creation API ```python -# Create a new object +# Create a new object — see [python-relationship-references.md] +# for the three accepted forms for relationship fields. obj = await self.client.create( kind="DcimDevice", data={ "name": "my-device", "status": "active", - # Use ID for relationships - "device_type": device_type_id, + # HFID dict for single-component HFID targets + "device_type": {"hfid": ["cEdge-1000"]}, } ) await obj.save(allow_upsert=True) @@ -71,7 +72,10 @@ ip = await self.client.allocate_next_ip_address( idempotent create-or-update - Use `self.client` (not `self._init_client`) inside `generate()` -- it has tracking enabled -- Use IDs for relationship references in `data` dict +- Reference related objects in `data` by HFID dict + (`{"hfid": ["name"]}`), ID dict (`{"id": ""}`), or SDK + object directly. A bare string is treated as ``id``, not HFID. + See [python-relationship-references.md](./python-relationship-references.md). - Handle empty/missing data gracefully -- GraphQL may return `None` for optional fields diff --git a/skills/infrahub-managing-generators/rules/python-multi-peer-add.md b/skills/infrahub-managing-generators/rules/python-multi-peer-add.md new file mode 100644 index 0000000..a575f41 --- /dev/null +++ b/skills/infrahub-managing-generators/rules/python-multi-peer-add.md @@ -0,0 +1,57 @@ +--- +title: Adding Multiple Peers to a Relationship +impact: HIGH +tags: python, relationship, add, members, manager, peers +--- + +## Adding Multiple Peers to a Relationship + +Impact: HIGH + +``RelationshipManager.add()`` takes **one peer per call**. Passing a +list of peers creates a single peer whose HFID is the list contents, +which the server rejects (or worse, accepts as malformed data). + +### How the bug looks + +The mutation generated by passing a list to ``.add()`` looks like: + +```text +members: [{hfid: [, , ]}] +``` + +— a single peer with a composite HFID equal to the entire list. The +server returns "Unable to find the node" or "Invalid HFID +component count" depending on the schema. + +### Anti-pattern + +```python +# WRONG — interprets the whole list as one peer +edges_to_add: list[CoreNode] = [d1, d2, d3] +group.members.add(edges_to_add) +await group.save() +``` + +### Correct pattern + +```python +# RIGHT — one peer per .add() call +edges_to_add: list[CoreNode] = [d1, d2, d3] +for peer in edges_to_add: + group.members.add(peer) +await group.save() +``` + +The same constraint applies to other ``RelationshipManager`` +mutators (``.update``, ``.remove``). + +### Why iterate, not collect into a list comprehension that's then passed to ``.add()``? + +``RelationshipManager.add(peer)`` mutates internal state in place; +the return value is not the new list. So +``[mgr.add(p) for p in peers]`` is equivalent to the correct pattern +above, but is harder to read and adds no value. Prefer the explicit +``for`` loop. + +Reference: [Infrahub SDK Docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-generators/rules/python-relationship-references.md b/skills/infrahub-managing-generators/rules/python-relationship-references.md new file mode 100644 index 0000000..1e90c83 --- /dev/null +++ b/skills/infrahub-managing-generators/rules/python-relationship-references.md @@ -0,0 +1,125 @@ +--- +title: Passing Relationships to client.create +impact: CRITICAL +tags: python, client, create, hfid, relationship, id +--- + +## Passing Relationships to client.create + +Impact: CRITICAL + +The SDK accepts four explicit forms for relationship fields in +the ``data=`` dict of ``client.create()``. Mixing them up — or +passing a bare string — is the most common source of "Unable to +find the node" errors at runtime. + +### The three accepted forms + +**Form A — HFID lookup (single-component).** Use when the target +schema's ``human_friendly_id`` is one field. The list contains +exactly one string. + +```python +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": {"hfid": ["cEdge-1000"]}, + "platform": {"hfid": ["ios-xe"]}, + }, +) +``` + +**Form B — HFID lookup (composite).** Use when +``human_friendly_id`` has multiple components. The list order +**must** match the schema's declared order. + +```python +# Schema: DcimInterface.human_friendly_id = [device__name__value, name__value] +await self.client.create( + kind="DcimInterface", + data={ + "name": "Ethernet1/1", + "device": {"hfid": ["spine-01"]}, + "connected_to": {"hfid": ["leaf-01", "Ethernet2/2"]}, + }, +) +``` + +**Form C — Explicit ID.** Use when you already hold the UUID +(returned by a prior query). + +```python +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "site": {"id": site_uuid}, + }, +) +``` + +**Form D — SDK object reference (passed directly).** Pass an +object previously returned by ``self.client.get`` or +``self.client.create``. + +```python +site = await self.client.get(kind="LocationSite", name__value="PAR-1") +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "site": site, + }, +) +``` + +### Anti-pattern: bare string + +A bare string for a relationship field is interpreted as +``{"id": ""}``. The server then tries to look up the +string as a UUID, fails, and returns "Unable to find the node". + +```python +# WRONG — server returns "Unable to find the node cEdge-1000 / DcimDeviceType" +await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": "cEdge-1000", # treated as id, not HFID + }, +) +``` + +### Anti-pattern: over-packed HFID list + +The HFID list length **must match** the schema's +``human_friendly_id`` declaration. Padding a single-component +HFID with extra fields causes the lookup to fail. + +```python +# WRONG — DcimDeviceType.human_friendly_id is [name__value], single-component. +# Adding manufacturer to the list breaks the lookup. +await self.client.create( + kind="DcimDevice", + data={ + "device_type": {"hfid": ["cEdge-1000", "Cisco"]}, # over-packed + }, +) +``` + +### How to know which form to use + +1. Find the target relationship's ``peer:`` kind in the schema. +2. Look up that node's ``human_friendly_id:`` list. The length + tells you how many strings go in the ``hfid`` list. +3. If you already hold a UUID, use ``{"id": ...}``. Otherwise + prefer ``{"hfid": [...]}``. +4. If you have an SDK object in scope (from ``client.get`` or + ``client.create``), pass it directly — Form D. + +The same forms apply to **every** relationship in the generator, +not only the examples above. If a relationship field doesn't fit +forms A-D, it is shape-wrong — fix it. + +Reference: [Infrahub Generator Docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-generators/rules/testing-integration.md b/skills/infrahub-managing-generators/rules/testing-integration.md new file mode 100644 index 0000000..4b26566 --- /dev/null +++ b/skills/infrahub-managing-generators/rules/testing-integration.md @@ -0,0 +1,47 @@ +--- +title: Run Generators End-to-End Before Declaring Done +impact: LOW +tags: testing, integration, infrahubctl, runtime, verification +--- + +## Run Generators End-to-End Before Declaring Done + +Impact: LOW + +Unit tests on the input dict do not cover SDK call shape. +Bug classes that pass unit tests but fail at runtime against a +real Infrahub server include: + +- HFID encoded as a bare string (treated as ``id``) +- Over-packed HFID list for a single-component target +- List passed to ``RelationshipManager.add`` instead of iterating +- Uniqueness collisions on bootstrap-seeded keys + +Every one of these has the same property: the Python code is +syntactically and type-wise fine; only the wire protocol shape is +wrong. **Run the generator against a live test instance before +declaring it done.** + +### Concrete workflow + +After the generator is implemented and unit-tested: + +1. ``infrahubctl generator list`` — confirm the new definition + registered. +2. ``infrahubctl generator run `` — execute + the generator against a real branch. +3. Verify the created objects exist via the UI or a GraphQL + query. Confirm relationships resolve. +4. If anything fails, fix and re-run before moving on. + +### When this matters most + +- Pre-PR self-review on a development branch. +- After any change to relationship reference shape. +- After any schema migration that changes ``human_friendly_id``. + +Unit tests are still valuable for ``clean_data()`` helpers, branch +logic, and pure-Python transforms — they just don't replace the +end-to-end run. + +Reference: [Infrahub Generator Docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-transforms/rules/_sections.md b/skills/infrahub-managing-transforms/rules/_sections.md index 547c647..eeaf4ad 100644 --- a/skills/infrahub-managing-transforms/rules/_sections.md +++ b/skills/infrahub-managing-transforms/rules/_sections.md @@ -19,7 +19,8 @@ 5. **Artifacts (artifacts-)** -- HIGH. Connecting transforms to output files via artifact_definitions, content types, targets (CoreArtifactTarget), - parameter mapping. + parameter mapping, and async regeneration polling + (the /api/artifact/generate endpoint is fire-and-forget). 6. **API Reference (api-)** -- HIGH. Class attributes (query, timeout), instance properties (client, @@ -31,3 +32,9 @@ 8. **Testing (testing-)** -- LOW. infrahubctl transform and render commands, REST API endpoints. + +9. **Queries (queries-)** -- CRITICAL. Writing the .gql + query that feeds the transform. Covers union-typed + relationships (DcimDevice.location, Organization*) that + require inline fragments (... on Type { fields }) to + avoid "Cannot query field 'X' on type 'Y'" errors. diff --git a/skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md b/skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md new file mode 100644 index 0000000..e666799 --- /dev/null +++ b/skills/infrahub-managing-transforms/rules/artifacts-async-regen-polling.md @@ -0,0 +1,100 @@ +--- +title: Programmatic Artifact Regeneration Is Fire-and-Forget +impact: HIGH +tags: artifacts, regenerate, async, polling, CoreArtifact +--- + +## Programmatic Artifact Regeneration Is Fire-and-Forget + +**Impact:** HIGH + +``POST /api/artifact/generate/?branch=`` returns +HTTP 200 as soon as the regen request is **queued**, not when +regen is **finished**. The async server-side regen can run +seconds later, and on a busy system can even run before a +recent generator's group-membership write is visible on the +read replica — at which point the regen sees the wrong target +set and silently produces nothing. + +Any caller that triggers regen programmatically (catalog page, +CI job, generator orchestration) **must poll** ``CoreArtifact`` +until the expected count materialises, with a hard timeout that +surfaces a warning instead of silently shipping zero artifacts. + +### Required shape of a programmatic regen helper + +Every function that triggers regen MUST contain all three: + +1. **A POST** to ``/api/artifact/generate/?branch=``. + Use whichever HTTP entry point fits — public methods like + ``client.post(...)``, ``httpx.post(...)``, ``requests.post(...)``, + ``aiohttp.ClientSession().post(...)``, or the infrahub_sdk's own + private helper ``client._post(url=..., payload={}, params=...)``. + The URL must contain the literal string + ``/api/artifact/generate``. +2. **A loop** (``while``, ``for``, or ``async for``) that waits + for completion. A function that POSTs and returns is the bug. +3. **A read** of ``CoreArtifact`` inside the loop — + ``client.filters(kind="CoreArtifact", ...)`` or equivalent — + to check whether regen has produced the expected count yet. + +If any of the three is missing, you have written the bug. Refer +to "Correct pattern" below. + +### Anti-pattern + +```python +async def regenerate(client, def_id, branch): + # WRONG — fire-and-forget; "success" doesn't mean anything finished + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + return "regenerated" +``` + +### Correct pattern + +```python +import asyncio + +POLL_INTERVAL_SECONDS = 2 +TIMEOUT_SECONDS = 60 + + +async def regenerate_and_wait(client, def_id, expected_count, branch): + """Trigger regen, poll until artifact count matches, or warn on timeout.""" + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + + deadline = asyncio.get_event_loop().time() + TIMEOUT_SECONDS + reposted = False + while asyncio.get_event_loop().time() < deadline: + artifacts = await client.filters( + kind="CoreArtifact", + definition__ids=[def_id], + branch=branch, + ) + if len(artifacts) >= expected_count: + return artifacts + if not reposted: + # Re-POST once: covers the read-replica visibility gap + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + reposted = True + await asyncio.sleep(POLL_INTERVAL_SECONDS) + + raise TimeoutError( + f"Artifact regen for {def_id} did not converge " + f"to {expected_count} within {TIMEOUT_SECONDS}s" + ) +``` + +### When this matters + +- **Catalog/UI pages** that trigger regen on user action — the + user clicks "deploy" and walks away; silent failure is the + worst outcome. +- **CI jobs** that gate a deploy on regen success. +- **Orchestrators** that chain generator → regen → assertion. + +Manual regen in the Infrahub UI doesn't need this — the UI +polls on its own. + +Reference: +[Infrahub artifacts docs](https://docs.infrahub.app) diff --git a/skills/infrahub-managing-transforms/rules/queries-union-fragments.md b/skills/infrahub-managing-transforms/rules/queries-union-fragments.md new file mode 100644 index 0000000..6c7e2a3 --- /dev/null +++ b/skills/infrahub-managing-transforms/rules/queries-union-fragments.md @@ -0,0 +1,94 @@ +--- +title: Querying Union-Typed Relationships with Inline Fragments +impact: CRITICAL +tags: gql, query, union, generic, inline-fragment, location +--- + +## Querying Union-Typed Relationships with Inline Fragments + +**Impact:** CRITICAL + +When a relationship's ``peer:`` in the schema is a **generic** +(or any type with multiple concrete inheritors that diverge in +fields), the GraphQL query must use inline fragments +(``... on TypeName { fields }``) to select fields that exist on +some inheritors but not others. Selecting such fields directly +on the union fails for any concrete instance whose type doesn't +define the field — the server returns +``Cannot query field 'X' on type 'Y'`` and the entire query +errors out. + +### Common union-typed relationships in Infrahub's base schema + +| Node.relationship | Peer (union) | Concrete inheritors | +| ----------------- | ------------ | ------------------- | +| ``DcimDevice.location`` | ``LocationGeneric`` | ``LocationSite``, ``LocationBuilding``, ``LocationHosting`` | +| ``OrganizationGeneric`` peers | ``OrganizationGeneric`` | ``OrganizationManufacturer``, ``OrganizationProvider``, ``OrganizationTenant`` | + +(Specifics vary by repo schema; always check ``peer:`` in +the schema before writing the query.) + +### Anti-pattern + +```graphql +query { + DcimDevice { + edges { + node { + location { + node { + name { value } # fails for LocationHosting + shortname { value } + } + } + } + } + } +} +``` + +Server response when the dataset contains a ``LocationHosting`` +instance: + +```text +Cannot query field 'name' on type 'LocationHosting'. +``` + +In a ``CoreRepository`` import, this stops the schema-sync; the +repo never finishes loading and downstream pipelines fail with +no obvious root cause. + +### Correct pattern + +```graphql +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + # LocationHosting has neither — explicitly skip, + # or include only fields it actually defines. + } + } + } + } + } +} +``` + +### How to know if a relationship needs fragments + +1. Find the relationship in the schema. Inspect ``peer:``. +2. If ``peer:`` is a generic type (or matches a ``generics:`` + entry by namespace+name), the relationship is a union. +3. Find every node with ``inherit_from:`` containing that + generic. Check whether each declares the same attribute + set. If they diverge, you need fragments. +4. When in doubt, use fragments. They never hurt; their + absence does. + +Reference: +[Infrahub schema docs](https://docs.infrahub.app) diff --git a/tests/graders/test_generators_lib.py b/tests/graders/test_generators_lib.py new file mode 100644 index 0000000..c00590c --- /dev/null +++ b/tests/graders/test_generators_lib.py @@ -0,0 +1,399 @@ +"""Tests for graders/managing-generators/lib.py AST helpers.""" + +import importlib.util +from pathlib import Path + +# Load the hyphenated-directory module by file path +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_LIB_PATH = _REPO_ROOT / "graders" / "managing-generators" / "lib.py" +_spec = importlib.util.spec_from_file_location( + "managing_generators_graders_lib", _LIB_PATH +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + +find_client_create_calls = _mod.find_client_create_calls +find_client_get_calls = _mod.find_client_get_calls +find_relationship_add_calls = _mod.find_relationship_add_calls +get_data_dict_items = _mod.get_data_dict_items +is_hfid_dict = _mod.is_hfid_dict +is_id_dict = _mod.is_id_dict +is_bare_string = _mod.is_bare_string +is_name_or_attribute = _mod.is_name_or_attribute +load_output_py = _mod.load_output_py + + +import ast + + +def _parse(src: str) -> ast.Module: + return ast.parse(src) + + +def test_find_client_create_calls_finds_await_self_client_create(): + tree = _parse( + "async def f():\n" + " await self.client.create(kind='D', data={'name': 'x'})\n" + ) + calls = find_client_create_calls(tree) + assert len(calls) == 1 + + +def test_find_client_create_calls_ignores_other_create_calls(): + tree = _parse( + "async def f():\n" + " await some.other.create(kind='D')\n" + " await self.client.update(kind='D')\n" + ) + assert find_client_create_calls(tree) == [] + + +def test_get_data_dict_items_returns_literal_dict_entries(): + tree = _parse( + "x = self.client.create(kind='D', data={'name': 'x', 'status': 'active'})\n" + ) + calls = [n for n in ast.walk(tree) if isinstance(n, ast.Call)] + items = get_data_dict_items(calls[0]) + assert set(items.keys()) == {"name", "status"} + + +def test_is_hfid_dict_matches_hfid_list_literal(): + node = ast.parse("x = {'hfid': ['cEdge-1000']}").body[0].value + ok, elts = is_hfid_dict(node) + assert ok is True + assert len(elts) == 1 + assert isinstance(elts[0], ast.Constant) + assert elts[0].value == "cEdge-1000" + + +def test_is_hfid_dict_rejects_non_dict_or_wrong_key(): + assert is_hfid_dict(ast.parse("x = 'foo'").body[0].value) == (False, None) + assert is_hfid_dict(ast.parse("x = {'id': '...'}").body[0].value) == (False, None) + + +def test_is_id_dict_matches_id_dict_literal(): + assert is_id_dict(ast.parse("x = {'id': 'abc'}").body[0].value) is True + assert is_id_dict(ast.parse("x = {'hfid': ['a']}").body[0].value) is False + + +def test_is_bare_string_matches_only_string_constants(): + assert is_bare_string(ast.parse("x = 'hi'").body[0].value) is True + assert is_bare_string(ast.parse("x = 42").body[0].value) is False + assert is_bare_string(ast.parse("x = ['a']").body[0].value) is False + + +def test_is_name_or_attribute_matches_variable_and_attribute_refs(): + assert is_name_or_attribute(ast.parse("x = device").body[0].value) is True + assert is_name_or_attribute(ast.parse("x = self.device").body[0].value) is True + assert is_name_or_attribute(ast.parse("x = 'd'").body[0].value) is False + + +def test_find_relationship_add_calls_finds_any_add(): + tree = _parse( + "def f():\n" + " group.members.add(device)\n" + " other.foo.add([1, 2])\n" + " plain.bar()\n" + ) + assert len(find_relationship_add_calls(tree)) == 2 + + +def test_find_client_get_calls_finds_self_client_get(): + tree = _parse( + "async def f():\n" + " x = await self.client.get(kind='D', name__value='x')\n" + " y = await self.client.create(kind='D')\n" + ) + assert len(find_client_get_calls(tree)) == 1 + + +def test_load_output_py_returns_none_for_missing_file(): + tree, raw = load_output_py(Path("/nonexistent/path.py")) + assert tree is None + assert raw == "" + + +def test_load_output_py_returns_none_tree_on_syntax_error(tmp_path): + p = tmp_path / "broken.py" + p.write_text("def f(:\n") + tree, raw = load_output_py(p) + assert tree is None + assert "def f" in raw + + +SRC_GOOD_HFID = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": {"hfid": ["cEdge-1000"]}, + "platform": {"hfid": ["ios-xe"]}, + }, + ) +""" + +SRC_BARE_STRING = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "device_type": "cEdge-1000", + }, + ) +""" + +SRC_OVERPACKED_HFID = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "device_type": {"hfid": ["cEdge-1000", "Cisco"]}, + }, + ) +""" + + +def test_relationship_hfid_form_correct_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["relationship-hfid-form-correct"](tree) + assert ok, msg + + +def test_relationship_hfid_form_correct_fails_on_bare_string(): + tree = ast.parse(SRC_BARE_STRING) + ok, msg = _mod.CHECKS["relationship-hfid-form-correct"](tree) + assert not ok + assert "device_type" in msg + + +def test_no_bare_string_relationship_fails_on_bare(): + tree = ast.parse(SRC_BARE_STRING) + ok, msg = _mod.CHECKS["no-bare-string-relationship"](tree) + assert not ok + + +def test_no_bare_string_relationship_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["no-bare-string-relationship"](tree) + assert ok + + +def test_no_overpacked_hfid_fails_on_overpacked(): + tree = ast.parse(SRC_OVERPACKED_HFID) + ok, msg = _mod.CHECKS["no-overpacked-hfid-list"](tree) + assert not ok + assert "device_type" in msg + + +def test_no_overpacked_hfid_passes_on_good(): + tree = ast.parse(SRC_GOOD_HFID) + ok, msg = _mod.CHECKS["no-overpacked-hfid-list"](tree) + assert ok + + +SRC_EMPTY_HFID = """ +async def f(self): + await self.client.create( + kind="DcimDevice", + data={ + "device_type": {"hfid": []}, + }, + ) +""" + + +def test_no_overpacked_hfid_fails_on_empty_hfid(): + tree = ast.parse(SRC_EMPTY_HFID) + ok, msg = _mod.CHECKS["no-overpacked-hfid-list"](tree) + assert not ok + assert "device_type" in msg + + +SRC_THREE_FORMS = """ +async def generate(self): + site = await self.client.get(kind="LocationSite", name__value="PAR-1") + device_type_uuid = "11111111-1111-1111-1111-111111111111" + await self.client.create( + kind="DcimDevice", + data={ + "name": "cEdge-1", + "manufacturer": {"hfid": ["Cisco"]}, + "device_type": {"id": device_type_uuid}, + "site": site, + }, + ) +""" + + +def test_hfid_form_for_name_lookup_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["hfid-form-for-name-lookup"](tree) + assert ok, msg + + +def test_id_form_for_uuid_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["id-form-for-uuid"](tree) + assert ok, msg + + +def test_sdk_object_reference_used_passes_on_three_forms(): + tree = ast.parse(SRC_THREE_FORMS) + ok, msg = _mod.CHECKS["sdk-object-reference-used"](tree) + assert ok, msg + + +def test_three_forms_fail_when_all_hfid(): + src = """ +async def generate(self): + await self.client.create( + kind="DcimDevice", + data={ + "name": "x", + "manufacturer": {"hfid": ["Cisco"]}, + "device_type": {"hfid": ["cEdge-1000"]}, + "site": {"hfid": ["PAR-1"]}, + }, + ) +""" + tree = ast.parse(src) + ok, _ = _mod.CHECKS["id-form-for-uuid"](tree) + assert not ok + ok, _ = _mod.CHECKS["sdk-object-reference-used"](tree) + assert not ok + + +# Task 4 follow-up: missing negative coverage for hfid-form-for-name-lookup +def test_hfid_form_for_name_lookup_fails_when_none_use_hfid(): + src = """ +async def generate(self): + site = await self.client.get(kind="LocationSite", name__value="PAR-1") + await self.client.create( + kind="DcimDevice", + data={ + "name": "x", + "device_type": {"id": "abc-uuid"}, + "site": site, + }, + ) +""" + tree = ast.parse(src) + ok, _ = _mod.CHECKS["hfid-form-for-name-lookup"](tree) + assert not ok + + +# Task 5 — Multi-peer add tests + +SRC_GOOD_ADD_LOOP = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + devices = [d1, d2, d3] + for peer in devices: + group.members.add(peer) + await group.save() +""" + +SRC_BAD_ADD_LIST = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + devices = [d1, d2, d3] + group.members.add(devices) + await group.save() +""" + +SRC_BAD_ADD_LIST_LITERAL = """ +async def generate(self): + group = await self.client.get(kind="CoreStandardGroup", name__value="g") + group.members.add([d1, d2, d3]) + await group.save() +""" + + +def test_no_list_passed_to_add_passes_on_iteration(): + tree = ast.parse(SRC_GOOD_ADD_LOOP) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert ok, msg + + +def test_no_list_passed_to_add_fails_on_list_literal(): + tree = ast.parse(SRC_BAD_ADD_LIST_LITERAL) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert not ok + + +def test_no_list_passed_to_add_fails_on_list_named_variable(): + tree = ast.parse(SRC_BAD_ADD_LIST) + ok, msg = _mod.CHECKS["no-list-passed-to-add"](tree) + assert not ok + + +def test_members_add_iterates_passes_on_loop(): + tree = ast.parse(SRC_GOOD_ADD_LOOP) + ok, msg = _mod.CHECKS["members-add-iterates"](tree) + assert ok, msg + + +def test_members_add_iterates_fails_on_no_loop(): + tree = ast.parse(SRC_BAD_ADD_LIST_LITERAL) + ok, msg = _mod.CHECKS["members-add-iterates"](tree) + assert not ok + + +def test_members_add_iterates_fails_on_different_paths(): + """Two .add() calls on different attribute paths shouldn't be treated as iteration.""" + src = """ +async def generate(self): + group.members.add(d1) + other.peers.add(d2) +""" + tree = ast.parse(src) + ok, msg = _mod.CHECKS["members-add-iterates"](tree) + assert not ok, f"Should not pass — different paths, no loop. Got: {msg}" + + +# Task 6 — Natural-key preflight tests + +SRC_PREFLIGHT = """ +from infrahub_sdk.exceptions import NodeNotFound + +async def create_prefix(client, user_prefix): + try: + existing = await client.get(kind="IpamPrefix", prefix__value=user_prefix) + return existing + except NodeNotFound: + pass + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save() +""" + +SRC_UPSERT = """ +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save(allow_upsert=True) +""" + +SRC_UNSAFE = """ +async def create_prefix(client, user_prefix): + prefix = await client.create(kind="IpamPrefix", data={"prefix": user_prefix}) + await prefix.save() +""" + + +def test_preflight_or_upsert_passes_on_preflight(): + tree = ast.parse(SRC_PREFLIGHT) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert ok + + +def test_preflight_or_upsert_passes_on_upsert(): + tree = ast.parse(SRC_UPSERT) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert ok + + +def test_preflight_or_upsert_fails_on_unsafe(): + tree = ast.parse(SRC_UNSAFE) + ok, _ = _mod.CHECKS["preflight-or-upsert"](tree) + assert not ok diff --git a/tests/graders/test_transforms_lib.py b/tests/graders/test_transforms_lib.py new file mode 100644 index 0000000..5c387f7 --- /dev/null +++ b/tests/graders/test_transforms_lib.py @@ -0,0 +1,396 @@ +"""Tests for graders/managing-transforms/lib.py helpers.""" + +import ast +import importlib.util +from pathlib import Path + + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_LIB_PATH = _REPO_ROOT / "graders" / "managing-transforms" / "lib.py" +_spec = importlib.util.spec_from_file_location( + "managing_transforms_graders_lib", _LIB_PATH +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) + + +find_inline_fragments = _mod.find_inline_fragments +block_for_relationship = _mod.block_for_relationship +field_appears_directly_under_union = _mod.field_appears_directly_under_union +has_post_to_artifact_generate = _mod.has_post_to_artifact_generate +has_loop_construct = _mod.has_loop_construct +references_core_artifact_in_call = _mod.references_core_artifact_in_call + + +# -- find_inline_fragments ------------------------------------------------- + + +def test_find_inline_fragments_basic(): + gql = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + fragments = find_inline_fragments(gql) + assert "LocationSite" in fragments + assert "LocationBuilding" in fragments + + +def test_find_inline_fragments_empty(): + assert find_inline_fragments("") == [] + assert find_inline_fragments("query { foo { bar } }") == [] + + +# -- block_for_relationship ----------------------------------------------- + + +def test_block_for_relationship_extracts_braced_block(): + gql = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } } } + } + } + } +} +""" + block = block_for_relationship(gql, "location") + assert block is not None + assert "node" in block + assert "name" in block + + +def test_block_for_relationship_missing_returns_none(): + gql = "query { DcimDevice { edges { node { name { value } } } } }" + assert block_for_relationship(gql, "location") is None + + +# -- field_appears_directly_under_union ----------------------------------- + + +def test_field_directly_under_union_detects_bug(): + bug_gql = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } shortname { value } } } + } + } + } +} +""" + assert field_appears_directly_under_union(bug_gql, "location", "name") is True + assert field_appears_directly_under_union(bug_gql, "location", "shortname") is True + + +def test_field_directly_under_union_passes_with_fragments(): + good_gql = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + # name is inside a fragment, not directly under location > node + assert field_appears_directly_under_union(good_gql, "location", "name") is False + + +def test_field_directly_under_union_missing_relationship(): + gql = "query { DcimDevice { edges { node { name { value } } } } }" + assert field_appears_directly_under_union(gql, "location", "name") is False + + +# -- has_post_to_artifact_generate ---------------------------------------- + + +def test_has_post_to_artifact_generate_httpx(): + tree = ast.parse( + "import httpx\n" + "r = httpx.post('https://infrahub/api/artifact/generate/abc?branch=main')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_requests(): + tree = ast.parse( + "import requests\n" + "r = requests.post(f'/api/artifact/generate/{def_id}?branch={br}')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_self_client(): + tree = ast.parse( + "async def f(self):\n" + " await self.client.post('/api/artifact/generate/xyz?branch=dev')\n" + ) + assert has_post_to_artifact_generate(tree) is True + + +def test_has_post_to_artifact_generate_no_match(): + tree = ast.parse("import httpx\nr = httpx.post('/api/other')\n") + assert has_post_to_artifact_generate(tree) is False + + +def test_has_post_to_artifact_generate_url_in_variable(): + """Fallback path: URL stored in a variable + a .post() call elsewhere. + + The strict AST helper can't reassemble the path through a Name node, + but the fallback (py_raw text + any .post call) accepts it. This is + a realistic pattern produced by capable LLMs. + """ + src = ( + "async def regenerate(client, def_id, branch):\n" + " endpoint = f'/api/artifact/generate/{def_id}?branch={branch}'\n" + " await client.post(endpoint)\n" + ) + tree = ast.parse(src) + # Without py_raw fallback, this fails (URL is a Name, not a Constant) + assert has_post_to_artifact_generate(tree) is False + # With py_raw, the fallback fires + assert has_post_to_artifact_generate(tree, src) is True + + +def test_has_post_to_artifact_generate_fallback_rejects_without_post(): + """The fallback requires SOME .post() call — text alone isn't enough.""" + src = ( + "URL_TEMPLATE = '/api/artifact/generate/{}'\n" + "# never actually POSTs\n" + ) + tree = ast.parse(src) + assert has_post_to_artifact_generate(tree, src) is False + + +def test_has_post_to_artifact_generate_sdk_private_post(): + """infrahub_sdk's private ``client._post(...)`` helper is matched.""" + src = ( + "async def regen(client, def_id, branch):\n" + " url = f'{client.address}/api/artifact/generate/{def_id}'\n" + " await client._post(url=url, payload={}, params={'branch': branch})\n" + ) + tree = ast.parse(src) + # Via fallback (url is a Name, not literal in the post call) + assert has_post_to_artifact_generate(tree, src) is True + + +def test_has_post_to_artifact_generate_sdk_private_post_literal_url(): + """``client._post(url="/api/artifact/generate/...")`` direct AST match.""" + src = ( + "async def regen(client):\n" + " await client._post(url='/api/artifact/generate/abc?branch=main')\n" + ) + tree = ast.parse(src) + assert has_post_to_artifact_generate(tree) is True + + +# -- has_loop_construct --------------------------------------------------- + + +def test_has_loop_construct_while(): + tree = ast.parse("while True:\n pass\n") + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_for(): + tree = ast.parse("for i in range(10):\n pass\n") + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_async_for(): + tree = ast.parse( + "async def f():\n" + " async for x in stream():\n" + " pass\n" + ) + assert has_loop_construct(tree) is True + + +def test_has_loop_construct_none(): + tree = ast.parse("x = 1\ny = 2\n") + assert has_loop_construct(tree) is False + + +# -- references_core_artifact_in_call ------------------------------------- + + +def test_references_core_artifact_filters_call(): + tree = ast.parse( + "x = client.filters(kind='CoreArtifact', definition__ids=[def_id])\n" + ) + assert references_core_artifact_in_call(tree) is True + + +def test_references_core_artifact_get_call(): + tree = ast.parse("x = client.get(kind='CoreArtifact', id=art_id)\n") + assert references_core_artifact_in_call(tree) is True + + +def test_references_core_artifact_no_match(): + tree = ast.parse("x = client.filters(kind='CoreNode')\n") + assert references_core_artifact_in_call(tree) is False + + +# -- Task 3: union-fragment checks ---------------------------------------- + + +SRC_BUG_QUERY = """ +query { + DcimDevice { + edges { + node { + location { node { name { value } shortname { value } } } + } + } + } +} +""" + +SRC_GOOD_QUERY = """ +query { + DcimDevice { + edges { + node { + location { + node { + ... on LocationSite { name { value } shortname { value } } + ... on LocationBuilding { name { value } } + } + } + } + } + } +} +""" + + +def test_query_uses_inline_fragments_for_location_passes_on_good(): + ok, msg = _mod.CHECKS["query-uses-inline-fragments-for-location"]( + gql_text=SRC_GOOD_QUERY, tree=None, py_raw="" + ) + assert ok, msg + + +def test_query_uses_inline_fragments_for_location_fails_on_bug(): + ok, msg = _mod.CHECKS["query-uses-inline-fragments-for-location"]( + gql_text=SRC_BUG_QUERY, tree=None, py_raw="" + ) + assert not ok + + +def test_query_no_direct_field_on_union_location_passes_on_good(): + ok, msg = _mod.CHECKS["query-no-direct-field-on-union-location"]( + gql_text=SRC_GOOD_QUERY, tree=None, py_raw="" + ) + assert ok, msg + + +def test_query_no_direct_field_on_union_location_fails_on_bug(): + ok, msg = _mod.CHECKS["query-no-direct-field-on-union-location"]( + gql_text=SRC_BUG_QUERY, tree=None, py_raw="" + ) + assert not ok + assert "name" in msg or "shortname" in msg + + +# -- Task 4: artifact regen polling checks -------------------------------- + + +SRC_GOOD_POLLING = """ +import asyncio + + +async def regen_and_wait(client, def_id, expected_count, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + deadline = asyncio.get_event_loop().time() + 60 + while asyncio.get_event_loop().time() < deadline: + artifacts = await client.filters( + kind="CoreArtifact", definition__ids=[def_id], branch=branch, + ) + if len(artifacts) >= expected_count: + return artifacts + await asyncio.sleep(2) + raise TimeoutError("regen did not converge") +""" + +SRC_BAD_FIRE_AND_FORGET = """ +async def regenerate(client, def_id, branch): + await client.post(f"/api/artifact/generate/{def_id}?branch={branch}") + return "regenerated" +""" + +SRC_BAD_NO_POST = """ +async def maybe_regen(client, def_id): + artifacts = await client.filters(kind="CoreArtifact", definition__ids=[def_id]) + return artifacts +""" + + +def test_posts_artifact_generate_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["posts-artifact-generate-endpoint"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_posts_artifact_generate_fails_when_missing(): + tree = ast.parse(SRC_BAD_NO_POST) + ok, msg = _mod.CHECKS["posts-artifact-generate-endpoint"]( + gql_text="", tree=tree, py_raw=SRC_BAD_NO_POST + ) + assert not ok + + +def test_has_polling_loop_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["has-polling-loop"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_has_polling_loop_fails_on_fire_and_forget(): + tree = ast.parse(SRC_BAD_FIRE_AND_FORGET) + ok, msg = _mod.CHECKS["has-polling-loop"]( + gql_text="", tree=tree, py_raw=SRC_BAD_FIRE_AND_FORGET + ) + assert not ok + + +def test_polls_coreartifact_passes_on_good(): + tree = ast.parse(SRC_GOOD_POLLING) + ok, msg = _mod.CHECKS["polls-coreartifact-after-post"]( + gql_text="", tree=tree, py_raw=SRC_GOOD_POLLING + ) + assert ok, msg + + +def test_polls_coreartifact_fails_on_fire_and_forget(): + tree = ast.parse(SRC_BAD_FIRE_AND_FORGET) + ok, msg = _mod.CHECKS["polls-coreartifact-after-post"]( + gql_text="", tree=tree, py_raw=SRC_BAD_FIRE_AND_FORGET + ) + assert not ok