From 096a89ae5bf432a6e2fde03d2d94657fa4139f4c Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 00:49:47 +0200 Subject: [PATCH 01/20] fix json output pollution to terminal, logging level set to waring, readme blank lines fix --- docsible/cli.py | 9 +- .../document_role/core_orchestrated.py | 7 + .../orchestrators/role_orchestrator.py | 7 +- docsible/diagrams/types/architecture.py | 36 +- .../processors/markdown_processor.py | 10 +- docsible/utils/special_tasks_keys.py | 32 +- ...uation-and-visualization-recommendation.md | 108 ++++ research/external-evaluation-candidates.md | 543 ++++++++++++++++++ tests/diagrams/test_architecture_diagram.py | 76 ++- tests/end_to_end/test_json_output_clean.py | 65 +++ tests/renderers/test_markdown_processor.py | 42 ++ tests/utils/test_special_tasks_keys.py | 87 +++ 12 files changed, 999 insertions(+), 23 deletions(-) create mode 100644 research/external-evaluation-and-visualization-recommendation.md create mode 100644 research/external-evaluation-candidates.md create mode 100644 tests/end_to_end/test_json_output_clean.py create mode 100644 tests/renderers/test_markdown_processor.py create mode 100644 tests/utils/test_special_tasks_keys.py diff --git a/docsible/cli.py b/docsible/cli.py index 5e48f14..9fe3b7e 100644 --- a/docsible/cli.py +++ b/docsible/cli.py @@ -26,18 +26,21 @@ def setup_logging(verbose: bool = False) -> None: """Configure logging for the application. + Logs are routed to stderr so stdout stays clean for data output + (e.g. machine-readable JSON from ``--output-format json``). + Args: - verbose: If True, set log level to DEBUG, otherwise INFO + verbose: If True, set log level to DEBUG, otherwise WARNING Example: >>> setup_logging(verbose=True) >>> logger.debug("This will be shown") """ - level = logging.DEBUG if verbose else logging.INFO + level = logging.DEBUG if verbose else logging.WARNING logging.basicConfig( level=level, format="%(levelname)s - %(message)s", - handlers=[logging.StreamHandler(sys.stdout)], + handlers=[logging.StreamHandler(sys.stderr)], ) diff --git a/docsible/commands/document_role/core_orchestrated.py b/docsible/commands/document_role/core_orchestrated.py index 12f192e..e963f53 100644 --- a/docsible/commands/document_role/core_orchestrated.py +++ b/docsible/commands/document_role/core_orchestrated.py @@ -6,6 +6,7 @@ import logging import os +import sys from pathlib import Path from typing import Any @@ -258,6 +259,12 @@ def doc_the_role(**kwargs: Any) -> None: apply_smart_defaults, ) + # In JSON mode, redirect logging to stderr so stdout carries only valid JSON + if kwargs.get("output_format") == "json": + for handler in logging.root.handlers: + if isinstance(handler, logging.StreamHandler) and handler.stream is sys.stdout: + handler.stream = sys.stderr + # SMART DEFAULTS INTEGRATION # Apply smart defaults based on role complexity (if enabled) enable_smart_defaults = os.getenv("DOCSIBLE_ENABLE_SMART_DEFAULTS", "true").lower() == "true" diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index a22662b..3104431 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -146,9 +146,12 @@ def execute(self) -> None: # Only show recommendations, don't generate documentation return - # Step 8: Handle dry-run mode + # Step 8: Handle dry-run mode (JSON mode carries machine output only) if self.context.processing.dry_run: - self._display_dry_run(role_info, role_path, analysis_report, diagrams, dependency_data) + if self.context.analysis.output_format != "json": + self._display_dry_run( + role_info, role_path, analysis_report, diagrams, dependency_data + ) return # Step 9: Render documentation diff --git a/docsible/diagrams/types/architecture.py b/docsible/diagrams/types/architecture.py index 61ab166..f84b831 100644 --- a/docsible/diagrams/types/architecture.py +++ b/docsible/diagrams/types/architecture.py @@ -114,11 +114,37 @@ def generate_component_architecture( if vars_count > 0: lines.append(f" vars --> {first_task_id}") - # Task file sequential flow (simplified - show first -> last) - if len(task_files) > 1: - first_id = f"tasks_{task_files[0].get('file', 'file0').replace('.', '_').replace('/', '_')}" - last_id = f"tasks_{task_files[-1].get('file', 'fileN').replace('.', '_').replace('/', '_')}" - lines.append(f" {first_id} --> {last_id}") + # Include/import flow between task files (statically resolvable targets only; + # templated targets are dynamic and are deliberately not drawn as edges) + task_file_ids = { + task_file.get("file", f"file{idx}"): ( + f"tasks_{task_file.get('file', f'file{idx}').replace('.', '_').replace('/', '_')}" + ) + for idx, task_file in enumerate(task_files) + } + task_file_ids_by_basename: dict[str, str] = {} + for file_name, node_id in task_file_ids.items(): + task_file_ids_by_basename.setdefault(file_name.split("/")[-1], node_id) + added_include_edges: set[tuple[str, str]] = set() + for task_file in task_files: + file_name = task_file.get("file", "") + source_id = task_file_ids.get(file_name) + if not source_id: + continue + for task in task_file.get("tasks", []): + target = task.get("include_target") + if not target or "{{" in target: + continue + target_id = task_file_ids.get(target) or task_file_ids_by_basename.get( + target.split("/")[-1] + ) + if ( + target_id + and target_id != source_id + and (source_id, target_id) not in added_include_edges + ): + lines.append(f' {source_id} -."includes".-> {target_id}') + added_include_edges.add((source_id, target_id)) # Tasks to handlers (notification) if task_files and handlers_count > 0: diff --git a/docsible/renderers/processors/markdown_processor.py b/docsible/renderers/processors/markdown_processor.py index 86706f9..706623d 100644 --- a/docsible/renderers/processors/markdown_processor.py +++ b/docsible/renderers/processors/markdown_processor.py @@ -34,15 +34,23 @@ def __init__( def process(self, markdown: str) -> str: """Validate and optionally auto-fix markdown formatting. + Excessive consecutive blank lines are always collapsed to the + validator's maximum, regardless of auto_fix. + Args: markdown: Raw markdown content Returns: - Fixed markdown (if auto_fix=True) or original markdown + Markdown with blank lines normalized; further fixes applied + only when auto_fix=True Raises: ValueError: If strict_validation=True and errors found """ + # Always enforce the validator's blank-line rule so generated output + # complies with docsible's own markdown validation by default. + markdown = self.markdown_fixer.fix_excessive_whitespace(markdown) + # Auto-fix if enabled (do this first) if self.auto_fix: original_markdown = markdown diff --git a/docsible/utils/special_tasks_keys.py b/docsible/utils/special_tasks_keys.py index 36dd8ee..2132cef 100644 --- a/docsible/utils/special_tasks_keys.py +++ b/docsible/utils/special_tasks_keys.py @@ -169,6 +169,7 @@ def process_special_task_keys( else: # Specific modules without 'action' key for key in ( + "include", "include_tasks", "import_tasks", "import_playbook", @@ -188,12 +189,27 @@ def process_special_task_keys( ] task_module = module_keys[0] if module_keys else "unknown" - tasks.append( - { - "name": escape_pipes(task_name), - "module": task_module if task_module != "unknown" else "", # Blank if unknown - "type": task_type, - "when": task_when, - } - ) + # Capture statically resolvable include/import targets for diagram edges + include_target: str | None = None + short_module = task_module.split(".")[-1] + if short_module in ("include", "include_tasks", "import_tasks", "include_role", "import_role"): + raw_target = task.get(task_module) + if isinstance(raw_target, str): + escaped_target = escape_pipes(raw_target) + if isinstance(escaped_target, str): + include_target = escaped_target + elif isinstance(raw_target, dict) and isinstance(raw_target.get("file"), str): + escaped_target = escape_pipes(raw_target["file"]) + if isinstance(escaped_target, str): + include_target = escaped_target + + processed_task: dict[str, Any] = { + "name": escape_pipes(task_name), + "module": task_module if task_module != "unknown" else "", # Blank if unknown + "type": task_type, + "when": task_when, + } + if include_target is not None: + processed_task["include_target"] = include_target + tasks.append(processed_task) return tasks diff --git a/research/external-evaluation-and-visualization-recommendation.md b/research/external-evaluation-and-visualization-recommendation.md new file mode 100644 index 0000000..87b44db --- /dev/null +++ b/research/external-evaluation-and-visualization-recommendation.md @@ -0,0 +1,108 @@ +# External Evaluation And Visualization Recommendation + +## Product Wording + +Docsible helps engineers understand and safely change unfamiliar Ansible roles by producing traceable documentation, quality findings, and navigable execution views. + +This positions the project around onboarding, migrations, handovers, and safe change rather than README generation alone. + +## External Evaluation Harness + +Build the executable external-evaluation harness outside this repository and outside the `docsible/` Python package. A separate repository or sibling workspace keeps third-party cloning, network access, licenses, changing remote branches, and evaluation-specific dependencies out of normal Docsible development and CI. + +Docsible should retain only stable machine-readable contracts that the harness consumes, such as JSON analysis output and documented CLI behavior. The harness should clone into temporary directories and never commit third-party source code. + +Use a pinned manifest for a small, diverse corpus. Each entry should record: + +- Repository URL and immutable commit SHA. +- License and role or collection path. +- Ansible features expected in the project. +- Stable assertions to evaluate after cloning. + +Assertions should focus on observable properties instead of full README snapshots: + +- Commands complete without crashes. +- Read-only commands do not write role files. +- JSON output is valid. +- Role, task, variable, handler, and include discovery is plausible for known fixtures. +- Generated Markdown validates. + +Run this harness manually, nightly, or before a release. Do not make normal pull requests depend on public network availability. + +### Location Decision + +The executable harness should be a sibling repository, for example +`docsible-evaluation`, rather than a module in this repository. This keeps +network access, third-party licensing, changing remote branches, and +evaluation-only dependencies separate from Docsible's normal development +workflow. This repository should retain the documented machine-readable +contracts that the harness consumes, not the cloned corpus. + +## Shortlisting Public Projects + +Do not clone every candidate before screening it. Start with repository metadata and a shallow, pinned checkout only after a candidate passes a shortlist review. + +Basic complexity signals are useful, but they are not enough on their own: + +- Number of roles, task files, tasks, defaults, handlers, and plugins. +- Presence of collections, FQCN modules, nested includes, imports, blocks, rescue/always, loops, conditions, notifications, templates, and argument specs. +- Repository activity, license clarity, testability, and absence of credentials or destructive automation concerns. + +Select for behavior diversity, not just high task counts. A useful initial corpus has roles that are simple, modular, collection-based, handler-heavy, include-heavy, condition-heavy, loop-heavy, and dynamically structured. Include at least one role whose graph is intentionally too large for a default Mermaid view. + +For every candidate, record a hypothesis before evaluation, for example: "Docsible should identify three task files, two handlers, and an include boundary." This prevents the harness from becoming a generic no-crash test. + +### Two-Stage Selection + +Use a candidate profile rather than a single complexity score. + +1. Discovery without a full clone: + - Use GitHub or Galaxy metadata for license, activity, repository size, and + language. + - Use a Git tree API or sparse shallow checkout to count roles, task files, + handlers, plugins, defaults, templates, and YAML files. + - Detect structural markers such as includes/imports, blocks, + rescue/always, loops, conditions, notifications, collections, FQCNs, and + argument specifications. +2. Pinned evaluation checkout: + - Select for feature diversity rather than task count alone. + - Pin an immutable commit SHA and clone shallowly into a temporary + directory. + - Never execute playbooks, repository scripts, or generated code. + +High task counts are a useful signal but not a sufficient selection rule. Ten +large roles with the same structure are less valuable than a deliberately +varied corpus that exercises distinct Ansible behaviors. + +## Human Value Evaluation + +The meaningful question is whether the outputs help engineers answer unfamiliar-role questions faster and more accurately. Compare a repository alone with the repository plus Docsible outputs for tasks such as: + +- What does this role install and configure? +- Which variables matter before a migration? +- Which task files and handlers are involved in a change? +- Which execution paths are static, conditional, or dynamic? + +Measure completion time, answer accuracy, and confidence with a small group of engineers. This is stronger evidence of value than graph size or README length. + +## Visualization Direction + +Mermaid remains appropriate for portable, small static summaries. It should not be the only execution view for complex roles. + +Build a renderer-independent execution and relationship graph model before integrating interactive visualization. The existing graph-visualization project can become a renderer adapter, not another parser or analyzer: + +```text +Ansible role -> role facts -> execution graph -> phase abstraction -> renderer + -> Mermaid summary + -> interactive view +``` + +Phase points are a useful readability abstraction when they group task flow into concepts such as install, configure, validate, migrate, start, and cleanup. The underlying graph must still preserve source links and uncertainty: + +- Statically resolvable includes link to their target files. +- Dynamic includes are marked dynamic or unknown. +- Loops remain one task node with loop metadata. +- Conditions annotate branches rather than pretending all paths execute. +- Handler notifications are explicit relationship edges. + +This yields an explorable model without claiming that Ansible execution is fully statically knowable. diff --git a/research/external-evaluation-candidates.md b/research/external-evaluation-candidates.md new file mode 100644 index 0000000..6f150a7 --- /dev/null +++ b/research/external-evaluation-candidates.md @@ -0,0 +1,543 @@ +# External Evaluation Candidates + +## Overview + +This document shortlists a small, diverse corpus of external public Ansible +projects for the docsible external evaluation harness, following the selection +philosophy in `external-evaluation-and-visualization-recommendation.md`. Every +candidate was screened without cloning, using the GitHub REST API (repo +metadata, head commit, recursive git tree) and raw file fetches pinned at an +immutable commit SHA. No repository was cloned and no playbook or repository +script was executed. + +The shortlist covers all required behavior-diversity categories: + +| Category | Anchor candidate(s) | +|---|---| +| Simple role | geerlingguy/ansible-role-docker, geerlingguy/ansible-role-nginx | +| Modular role (task files split by concern) | nginx/ansible-role-nginx, geerlingguy/ansible-role-mysql | +| Collection-based project (galaxy.yml) | prometheus-community/ansible, dev-sec/ansible-collection-hardening | +| Handler-heavy (many handlers / notify chains) | ansible-lockdown/UBUNTU22-CIS (40 handlers, 139 notify lines) | +| Include-heavy (include/import boundaries) | ansible-lockdown/UBUNTU22-CIS (96 boundaries, 68 task files) | +| Condition-heavy (when, blocks) | ansible-lockdown/UBUNTU22-CIS (826 when lines, 189 blocks), openstack/ansible-hardening | +| Loop-heavy (loop, loop_var, with_*) | geerlingguy/ansible-role-mysql, dev-sec/ansible-collection-hardening | +| Dynamically structured (templated includes, include_role) | nginx/ansible-role-nginx, openstack/ansible-hardening, prometheus-community/ansible | +| Graph too large for a default Mermaid view | ansible-lockdown/UBUNTU22-CIS (~925 tasks) | + +Two known gaps remain and are recorded under Limitations: no shortlisted repo +uses `rescue:`/`always:` (0 occurrences across the entire corpus), and the +legacy `collections:` keyword is absent everywhere (0 occurrences). + +## Selection method + +Stage-one screening without cloning, per the two-stage selection policy: + +1. `GET https://api.github.com/repos/OWNER/REPO` for description, stars, + license, default branch, `pushed_at`, and archived status. +2. `GET https://api.github.com/repos/OWNER/REPO/commits/BRANCH` for the + immutable head commit SHA to pin. +3. `GET https://api.github.com/repos/OWNER/REPO/git/trees/SHA?recursive=1` + for full file listings (all trees returned `truncated: false`). +4. Raw fetches (`https://raw.githubusercontent.com/OWNER/REPO/SHA/path`) of + every `tasks/` and `handlers/` YAML file, plus spot checks of LICENSE and + defaults files. Raw fetches are not rate limited by the API quota. + +Counts in this document are derived as follows and are reproducible at the +pinned SHAs: role/task/handler/template/molecule file counts from git trees; +task counts as `^\s*- name:` grep entries across raw task files (a lower +bound; nameless tasks are not counted); handler counts as `- name:` entries +in handlers files; structural markers (include_tasks, import_tasks, blocks, +loops, when, notify, FQCN) as grep line counts across the same raw files. +Grep line counts are approximate by nature — see Limitations. + +Candidate table (screened 2026-09-12): + +| Repo | License | Pin SHA | Size signals | Category | +|---|---|---|---|---| +| geerlingguy/ansible-role-docker | MIT | 38be616950679548ae0ba8a81ffcceca1b3090bd | 6 task files, ~41 tasks, 2 handlers | simple role | +| geerlingguy/ansible-role-nginx | MIT | 5ff0b235006390a0d5666fd4cce7477410982cdf | 9 task files, ~24 tasks, 3 handlers | simple role, OS-split includes | +| geerlingguy/ansible-role-mysql | MIT | 0a0ea6b728120b3ab3918332d9404bb65836834d | 10 task files, ~52 tasks, 1 handler | modular role, legacy loops | +| geerlingguy/ansible-role-postgresql | MIT | 53abdf144de8231b2f2ce0652523eebc3eda7100 | 10 task files, ~38 tasks, 27 vars files | modular role, include+import mix | +| nginx/ansible-role-nginx | Apache-2.0 | 157e0e97406f798bd6f50db37430a78c4269aa92 | 31 task files (nested), ~244 tasks, 8 handlers | modular, handler-heavy, dynamic include | +| prometheus-community/ansible | Apache-2.0 | e2f46e17d33651c3c09042aaa9c8f29b87a9753f | 25 roles, 65 task files, ~383 tasks, 31 handlers | collection, include_role x100 | +| dev-sec/ansible-collection-hardening | Apache-2.0 | 3102eddbd116c5f8c1581aca543d372dbc326764 | 4 roles, 37 task files, ~217 tasks, 9 handlers | collection, condition/loop-heavy | +| ansible-lockdown/UBUNTU22-CIS | MIT | fad97b54d843eaffc4b7790686cc88bbcbb2330e | 68 task files, ~925 tasks, 40 handlers | too-large graph, include/condition/handler-heavy | +| elastic/ansible-elasticsearch | Apache-2.0 | af05c6470ef63337deba7009eec6af3ea05e2193 | 20 task files, ~182 tasks, 2 handlers | legacy syntax (bare include:, no FQCN) | +| openstack/ansible-hardening | Apache-2.0 | 8c3b5fcf06bf8f93f0f275934af40a27ab44431d | 20 task files, ~193 tasks, 8 handlers | condition-heavy, templated includes | + +## Detailed candidate profiles + +### geerlingguy/ansible-role-docker (simple role) + +- URL: https://github.com/geerlingguy/ansible-role-docker +- Pin: master @ 38be616950679548ae0ba8a81ffcceca1b3090bd +- License: MIT (source: `GET /repos/geerlingguy/ansible-role-docker`, 2026-09-12) +- Stats: 2291 stars, pushed 2026-08-18 (source: same endpoint) +- Structure: root-level standalone role. 6 task files + (`tasks/main.yml`, `setup-Debian.yml`, `setup-RedHat.yml`, `setup-Suse.yml`, + `docker-compose.yml`, `docker-users.yml`), 1 handler file (2 handlers), + 1 defaults file (~29 top-level keys), 6 vars files, 4 molecule tree paths + (source: git tree at pinned SHA). +- Markers (raw fetch of all 6 task files + handlers at pinned SHA): + 5 static `include_tasks` (OS setup, compose, users), 2 blocks, 0 + rescue/always, 33 `when` lines, 2 `with_items` lines, 0 `loop:`, + 11 `ansible.builtin.*` FQCN lines, no `collections:` keyword. +- Hypothesis: docsible discovers 6 task files, ~41 tasks, 2 handlers, 5 + include boundaries, 1 defaults file with ~29 variables; the execution graph + fits a default Mermaid view and `analyze role --output-format json` parses. +- Risks: none significant; the de facto most-cloned reference role, ideal + baseline for discovery-count calibration. + +### geerlingguy/ansible-role-nginx (simple role, OS-split includes) + +- URL: https://github.com/geerlingguy/ansible-role-nginx +- Pin: master @ 5ff0b235006390a0d5666fd4cce7477410982cdf +- License: MIT (source: `GET /repos/geerlingguy/ansible-role-nginx`) +- Stats: 894 stars, pushed 2026-08-10 (same source) +- Structure: 9 task files (`main.yml` + 7 `setup-.yml` + `vhosts.yml`), + 3 handlers, 3 templates, 8 vars files, 3 molecule tree paths + (source: git tree at pinned SHA). +- Markers (raw fetch of all task/handler files): 7 `include_tasks` (static, + OS-guarded) + 1 `import_tasks` (`vhosts.yml`), 18 `when` lines, 2 + `with_items`, 0 blocks, 0 rescue/always, no FQCN (short module names). +- Hypothesis: docsible discovers 9 task files, ~24 tasks, 3 handlers, 8 + include/import boundaries; short module names (no FQCN) must render without + crashing. +- Risks: none significant. Overlaps the simple-role category with the docker + role; keep for the import/include mix and the no-FQCN rendering path. + +### geerlingguy/ansible-role-mysql (modular role, legacy loops) + +- URL: https://github.com/geerlingguy/ansible-role-mysql +- Pin: master @ 0a0ea6b728120b3ab3918332d9404bb65836834d +- License: MIT (source: `GET /repos/geerlingguy/ansible-role-mysql`) +- Stats: 1130 stars, pushed 2026-07-31 (same source) +- Structure: 10 task files split by concern (`configure.yml`, + `databases.yml`, `replication.yml`, `secure-installation.yml`, `users.yml`, + `variables.yml`, `setup-.yml` x3), 1 handler, 3 templates, 12 vars + files, defaults with ~63 top-level keys, 4 molecule tree paths + (source: git tree + raw defaults at pinned SHA). +- Markers (raw fetch of all 10 task files + handlers): 9 + `ansible.builtin.include_tasks` boundaries (static filenames), 1 block, + 38 `when` lines, 11 legacy loop lines (9 `with_items`, 2 + `with_first_found`), 0 modern `loop:`, 49 `ansible.builtin.*` + 2 + `community.mysql.*` FQCN lines. +- Hypothesis: docsible discovers 10 task files, ~52 tasks, 9 include + boundaries, 1 handler, ~63 defaults variables; `with_items` loop metadata + renders as loop info on the task nodes. +- Risks: none significant. + +### geerlingguy/ansible-role-postgresql (modular role, include+import mix) + +- URL: https://github.com/geerlingguy/ansible-role-postgresql +- Pin: master @ 53abdf144de8231b2f2ce0652523eebc3eda7100 +- License: MIT (source: `GET /repos/geerlingguy/ansible-role-postgresql`) +- Stats: 655 stars, pushed 2026-09-08 (same source) +- Structure: 10 task files, 1 handler, 2 templates, 27 vars files + (per-distribution/version), defaults with ~16 top-level keys, 3 molecule + tree paths (source: git tree at pinned SHA). +- Markers (raw fetch of all 10 task files + handlers): 6 `include_tasks` + (OS setup) + 3 `import_tasks` (`users.yml`, `databases.yml`, + `users_props.yml`), 21 `when` lines, 9 `with_items`, 0 blocks. +- Hypothesis: docsible discovers 10 task files, 27 vars files, 6 include + 3 + import boundaries; the variables table handles a 27-file vars directory + without truncation errors. +- Risks: overlaps mysql (same author, same loop style). Kept because the + import/include mix and vars-file count differ materially. + +### nginx/ansible-role-nginx (modular role, handler-heavy, dynamic include) + +- URL: https://github.com/nginx/ansible-role-nginx + (successor of the moved `nginxinc/ansible-role-nginx`; source: + `GET /repositories/117019566` returned `nginx/ansible-role-nginx`) +- Pin: main @ 157e0e97406f798bd6f50db37430a78c4269aa92 +- License: Apache-2.0 (source: `GET /repositories/117019566` and + `GET /repos/nginx/ansible-role-nginx`) +- Stats: 704 stars, pushed 2026-09-09, not archived (same source) +- Structure: standalone role with galaxy.yml (published as `nginxinc.nginx`). + 31 task files in nested concern directories (`tasks/agent/`, `config/`, + `keys/`, `modules/`, `opensource/`, `plus/`, `prerequisites/`, `validate/`), + 1 handler file (8 handlers), 7 defaults files, 6 templates, 66 molecule + tree paths (source: git tree at pinned SHA). +- Markers (raw fetch of all 31 task files + handlers): 21 `include_tasks` + including at least one templated filename + (`include_tasks: "{{ role_path }}/tasks/opensource/install-{{ ansible_facts['os_family'] | lower }}.yml"`) + that cannot be statically resolved; 38 blocks, 0 rescue/always; 162 `when` + lines; 8 `loop:` and 0 `with_*`; 28 `notify` lines; 173 `ansible.builtin.*` + + 51 `community.*` FQCN lines. +- Hypothesis: docsible discovers 31 task files across nested directories, + ~244 tasks, 8 handlers, 21 include boundaries with at least 1 marked + dynamic/unresolvable; notify edges connect tasks to 8 handlers. +- Risks: the templated include means the true execution graph is open-ended; + assertions must accept a dynamic/unknown node rather than a resolved file. + +### prometheus-community/ansible (collection, include_role dynamics) + +- URL: https://github.com/prometheus-community/ansible +- Pin: main @ e2f46e17d33651c3c09042aaa9c8f29b87a9753f +- License: Apache-2.0 (source: `GET /repos/prometheus-community/ansible`) +- Stats: 577 stars, pushed 2026-09-11, actively maintained (same source) +- Structure: the `prometheus.prometheus` collection. galaxy.yml present; + 25 public roles (`alertmanager`, `node_exporter`, `prometheus`, 22 more) + plus a hidden `roles/_common` role; 65 role task files, ~383 tasks, 26 + handler files (31 handlers), 25 defaults files, 26 `meta/argument_specs.yml` + files (one per role plus `_common`), 312 molecule tree paths, 36 templates + (source: git tree at pinned SHA). +- Markers (raw fetch of all 91 role task/handler files): 36 `include_tasks`; + 100 `ansible.builtin.include_role` lines, all targeting + `name: prometheus.prometheus._common` with `tasks_from:` + (`preflight.yml`, `install.yml`, `selinux.yml`); 23 blocks, 0 rescue/always; + 162 `when` lines; 9 `loop:` + 7 `with_*` lines (5 `with_items`, 1 + `with_fileglob`, 1 `with_dict`); 25 `notify` lines; 419 + `ansible.builtin.*` FQCN lines. +- Hypothesis: `docsible scan collection .` discovers 25 roles (or 26 if + `_common` is counted — first calibration run must pin which number is + correct and record it as the assertion); `document role --collection` + produces per-role output; every role has argument_specs metadata; the 100 + `include_role` boundaries are marked dynamic or resolved to `_common` + tasks_from targets. +- Risks: `_common` is underscore-prefixed internal content; whether docsible + counts it is exactly the kind of contract the harness should pin. Very + active repo — re-pin SHA for each harness run. + +### dev-sec/ansible-collection-hardening (collection, condition/loop-heavy) + +- URL: https://github.com/dev-sec/ansible-collection-hardening +- Pin: master @ 3102eddbd116c5f8c1581aca543d372dbc326764 +- License: Apache-2.0 (source: `GET /repos/dev-sec/ansible-collection-hardening`) +- Stats: 5470 stars, pushed 2026-09-08 (same source) +- Structure: the `devsec.hardening` collection. galaxy.yml present; 4 roles + (`mysql_hardening`, `nginx_hardening`, `os_hardening`, `ssh_hardening`); + 37 role task files, ~217 tasks, 4 handler files (9 handlers), 24 templates, + 30 role vars files, argument_specs in all 4 roles, 79 molecule tree paths + (source: git tree at pinned SHA). +- Markers (raw fetch of all 41 role task/handler files): 11 `include_tasks` + + 22 `import_tasks`; 6 blocks, 0 rescue/always; 143 `when` lines; 29 + modern `loop:` + 14 legacy `with_*` lines including 4 + `with_community.general.flattened` (FQCN lookup loop), 4 `with_dict`, 3 + `with_first_found`, 2 `with_items`, 1 `with_subelements`; 2 `loop_var` + usages; 20 `notify` lines; 200 `ansible.builtin.*` + 9 `community.*` FQCN + lines. +- Hypothesis: docsible discovers 4 roles, each with argument_specs; 22 + import + 11 include boundaries; `loop_var` metadata appears on at least 2 + tasks; both modern `loop:` and legacy `with_*` render as loop info. +- Risks: none significant. The only shortlist repo exercising `loop_var`, + `with_subelements`, and FQCN-prefixed lookup loops. + +### ansible-lockdown/UBUNTU22-CIS (too-large graph, include/condition/handler-heavy) + +- URL: https://github.com/ansible-lockdown/UBUNTU22-CIS +- Pin: devel @ fad97b54d843eaffc4b7790686cc88bbcbb2330e + (note: default branch is `devel`, not main/master) +- License: MIT (source: `GET /repos/ansible-lockdown/UBUNTU22-CIS` says MIT; + verified against raw + `https://raw.githubusercontent.com/ansible-lockdown/UBUNTU22-CIS/fad97b54d843eaffc4b7790686cc88bbcbb2330e/LICENSE`, + "MIT License, Copyright (c) 2026 MindPoint Group - A Tyto Athene Company / + Ansible Lockdown") +- Stats: 257 stars, pushed 2026-08-25 (same API source) +- Structure: root-level standalone role. 68 task files: 11 directly under + `tasks/` (`main.yml`, `prelim.yml`, `post.yml`, audit plumbing) plus 57 + nested under `tasks/section_1/` .. `tasks/section_7/`; 1 handler file with + 40 handlers; `defaults/main/` is a directory (`main.yml` ~532 top-level + keys plus `audit.yml`); 42 templates; 13 molecule tree paths (source: git + tree at pinned SHA; defaults keys counted from raw fetch). +- Markers (raw fetch of all 68 task files + handlers): 95 `import_tasks` + 1 + `include_tasks` boundaries; 189 blocks, 0 rescue/always; 826 `when` lines; + 76 `loop:` + ~20 `with_*` lines (17 `with_items`; 3 other matches were + false positives — see Limitations); 139 `notify` lines (handler chains); + 83 `loop_control: label:` usages; 739 `ansible.builtin.*` + 20 + `community.*` FQCN lines. +- Hypothesis: docsible discovers 68 task files, ~925 tasks, 40 handlers, 96 + import/include boundaries; `document role --graph` and `analyze role` + must complete without crashing and the JSON `truncated` field or diagram + simplification must engage for a graph of this size; a default Mermaid + view of ~925 nodes is intentionally too large and is the stress case for + the "too large for default Mermaid" requirement. +- Risks: benchmark roles are restructured frequently between benchmark + releases — pin strictly by SHA. Destructive remediation content + (auditd, PAM, SSH changes) is irrelevant to static analysis but the + harness must never execute it. + +### elastic/ansible-elasticsearch (legacy syntax: bare include:, no FQCN) + +- URL: https://github.com/elastic/ansible-elasticsearch +- Pin: main @ af05c6470ef63337deba7009eec6af3ea05e2193 +- License: Apache-2.0 (source: raw + `https://raw.githubusercontent.com/elastic/ansible-elasticsearch/main/LICENSE`; + the API license field reports NOASSERTION/"Other" — see Limitations) +- Stats: 1588 stars, archived, last push 2022-06-24 (source: + `GET /repos/elastic/ansible-elasticsearch`) +- Structure: root-level standalone role. 20 task files: 14 directly under + `tasks/` plus 6 nested under `tasks/xpack/` and `tasks/xpack/security/`; + 1 handler file (2 handlers); 7 templates; defaults with ~62 top-level + keys; 3 vars files; no molecule (source: git tree + raw fetch at pinned + SHA). +- Markers (raw fetch of all 20 task files + handlers): 19 legacy bare + `include:` statements (static filenames, no `include_tasks`/ + `import_tasks`); 0 FQCN anywhere (pre-2.10 short module names); 24 legacy + loop lines (22 `with_items`, 2 `with_fileglob`); 6 blocks; 142 `when` + lines; 17 `notify` lines. +- Hypothesis: docsible parses the legacy `include:` keyword as an include + boundary (19 boundaries), renders short module names without crashing, + and discovers 20 task files including the nested `xpack/security/` + subtree. +- Risks: archived since 2022 — acceptable because the pinned SHA is + immutable and the corpus value is the legacy syntax coverage, but it will + never receive fixes; if the harness needs a maintained legacy-style role, + none was found during screening (see Limitations). + +### openstack/ansible-hardening (condition-heavy, templated includes) + +- URL: https://github.com/openstack/ansible-hardening +- Pin: master @ 8c3b5fcf06bf8f93f0f275934af40a27ab44431d +- License: Apache-2.0 (source: `GET /repos/openstack/ansible-hardening`; + repo is a mirror of opendev.org) +- Stats: 688 stars, pushed 2026-08-27 (same source) +- Structure: root-level standalone role. 20 task files: `tasks/main.yml`, + `tasks/contrib/main.yml`, and 18 files under `tasks/rhel7stig/`; 1 handler + file (8 handlers); 13 templates; 5 vars files (`main`, `debian`, + `redhat-8/9/10`) (source: git tree at pinned SHA). +- Markers (raw fetch of all 20 task files + handlers): 13 `import_tasks` + + 4 `include_tasks`, of which two have templated, statically unresolvable + filenames: `ansible.builtin.import_tasks: "{{ stig_version }}stig/main.yml"` + and `ansible.builtin.include_tasks: "{{ ansible_facts['pkg_mgr'] }}.yml"`; + 8 blocks; 201 `when` lines; 20 legacy loop lines (19 `with_items`, 1 + `with_nested`); 1 modern `loop:`; 20 `notify` lines; 187 + `ansible.builtin.*` FQCN lines. +- Hypothesis: docsible discovers 20 task files, ~193 tasks, 8 handlers, 17 + include/import boundaries of which exactly 2 are marked dynamic/ + unresolvable; the graph renders conditional branches rather than + pretending all paths execute. +- Risks: `{{ stig_version }}` resolves to `rhel7stig` in practice via + vars/defaults, so a smarter future analyzer could resolve it; the + assertion should be "completes and marks it dynamic or resolves it via + defaults", not a hard failure either way. + +## Rejected candidates and why + +- ansible-collections/community.general (GPL-3.0-or-later, 1068 stars). + No `roles/` directory — top-level content is plugins, tests, docs, meta, + changelogs (source: repository file listing fetched from + https://github.com/ansible-collections/community.general on 2026-09-12; + API license field GPL-3.0). Docsible needs roles; a module-only collection + exercises no role-documentation behavior. +- ansible-collections/kubernetes.core (GPL-3.0, 261 stars). Same reason, + verified harder: full git tree at head + a7922e498ca8fa62aef89b47174868f0d8d0ae7d (810 entries) contains 22 + `plugins/modules/*.py` and zero `roles/` paths (source: + `GET /git/trees/...?recursive=1`). License verified from raw `LICENSE` + (GPL-3.0 text; API field said NOASSERTION). +- ansible-collections/community.docker, ansible-collections/ansible.posix, + ansible-collections/community.postgresql. No `roles/` paths found in the + repository file listings (HTML payload grep of each repo page, + 2026-09-12). ansible.posix and community.postgresql licenses verified + from raw `COPYING` files as GPL-3.0 (API fields said NOASSERTION). +- nginxinc/ansible-role-nginx. HTTP 301 "Moved Permanently" (source: + `GET /repos/nginxinc/ansible-role-nginx` returned a redirect to + `/repositories/117019566`, which resolves to `nginx/ansible-role-nginx`). + The successor repo is shortlisted. +- cloudalchemy/ansible-prometheus (MIT, 1114 stars). Archived 2023-03-06 + (source: `GET /repos/cloudalchemy/ansible-prometheus`). Its exporter roles + were migrated into prometheus-community/ansible, which is shortlisted; + keeping both would duplicate the same role families. +- atosatto/ansible-role-elasticsearch. 404 Not Found (source: + `GET /repos/atosatto/ansible-role-elasticsearch`). Renamed or removed. +- mongo_single. Could not be identified as a well-known public Ansible role. + GitHub search `q=mongo_single in:name` returned only unrelated JavaScript + coursework repositories and unlicensed personal projects (source: + `GET /search/repositories?q=mongo_single+in:name`, 2026-09-12). Rejected + as unverifiable rather than risk an unmaintained, unlicensed corpus entry. +- geerlingguy docker_ce collection. Does not exist under that name. Search + `q=docker_ce user:geerlingguy` returns only `docker-centos6/7/8-ansible` + playbook repos (source: `GET /search/repositories?q=docker_ce+user:geerlingguy`). +- geerlingguy/ansible-role-jenkins (MIT, 851 stars, active; source: + `GET /repos/geerlingguy/ansible-role-jenkins`). Screened but not + shortlisted: the handler-heavy and modular categories are already covered + by stronger anchors, and the corpus budget is 10. +- elastic/ansible-elasticsearch was nearly rejected for archival but kept + deliberately: it is the only screened repo still using the legacy bare + `include:` keyword, zero FQCN, and `with_items` loops throughout. Pinned + SHAs are immutable, so archival is acceptable for a static-analysis corpus. + +## Evaluation harness proposal + +Sibling repository `docsible-evaluation` (outside this repo and the +`docsible/` package), per the location decision in the recommendation doc. +It contains only: a pinned corpus manifest, a runner script, and recorded +results. No third-party source is committed. + +Pinned manifest (YAML, one file per candidate or a single corpus file): + +```yaml +- id: ubuntu22-cis + url: https://github.com/ansible-lockdown/UBUNTU22-CIS + sha: fad97b54d843eaffc4b7790686cc88bbcbb2330e + license: MIT + kind: role + path: . + categories: [too-large-graph, include-heavy, condition-heavy, handler-heavy] + features: [import_tasks, include_tasks, blocks, when, loop, with_items, notify, loop_control] + hypotheses: + - command: docsible analyze role --role . --output-format json + asserts: + - exit_code: 0 + - json_parses: true + - json_schema: [role, findings, summary, truncated] + - task_files_discovered: 68 + - handlers_discovered: 40 + - include_boundaries: 96 + - tasks_discovered: {min: 850, max: 1000} + - no_mutation: git-status-empty + notes: default branch is devel; pin by SHA only +``` + +Two-stage selection (as proven during this research): + +1. Screening without cloning: `GET /repos/OWNER/REPO`, + `GET /repos/OWNER/REPO/git/trees/HEAD?recursive=1`, plus raw fetches of + representative task files to confirm structural markers. Unauthenticated + API allows ~20 screened repos per hour per IP (60 requests); a CI token + raises this to 5000/h but manual local runs should work unauthenticated + with cached metadata. +2. Pinned evaluation checkout: `git fetch --depth 1 origin ` into a + `mktemp` directory, checkout `FETCH_HEAD`, run docsible commands, assert, + delete. Never execute playbooks, repo scripts, or CI configs from the + corpus. Never commit corpus code. + +Assertion types: + +- Command completes: exit code 0, no traceback, for `analyze role`, + `validate role`, `scan collection`, and `document role --dry-run`. +- No mutation: after every read-only command, `git status --porcelain` in + the cloned tree must be empty. For `document role` (writes README), assert + only expected output files change and that `--no-backup` / backup behavior + matches documented flags. +- JSON validity: `--output-format json` parses and conforms to the + documented schema (`role`, `findings[]`, `summary{total, critical, + warning, info}`, `truncated`). +- Discovery counts match hypotheses: exact counts where the parser contract + is unambiguous (files, handlers), tolerant ranges where grep-vs-parser + semantics differ (task counts; see Limitations). First run against each + candidate is a calibration run: record actuals, then freeze them as + assertions so the harness is not a generic no-crash test. +- Markdown exists and validates: `document role` produces a README that + passes `validate role --strict-validation`. +- Graph behavior: for UBUNTU22-CIS, the graph path must complete and engage + truncation/simplification for ~925 tasks; assert completion and valid + JSON, not an exact node count. + +Cadence: run manually, nightly, and before releases in the sibling repo. +Never on normal docsible PRs; never as a gate that depends on public network +availability. + +Human value evaluation sketch: 4-6 engineers, two corpus roles (one simple, +e.g. docker; one unfamiliar and complex, e.g. a UBUNTU22-CIS section or +devsec os_hardening). Each answers the philosophy-doc questions (what does +this role install/configure; which variables matter; which files and +handlers are involved; which paths are static, conditional, dynamic) under +two conditions: repository only, then repository plus docsible outputs. +Measure completion time, answer accuracy (scored against the pinned SHA's +actual structure), and self-reported confidence. A compact scorecard per +participant per role is enough; no tooling required beyond a timer. + +## Limitations encountered during research + +- GitHub API rate limit: never hit the wall, but the budget shaped the + method. Final state per response header `x-ratelimit-remaining`: 18 of 60 + unauthenticated core requests left (42 used) at the end of research on + 2026-09-12. Screening was therefore capped at ~18 metadata fetches plus + 11 commit/tree pairs; every response was cached to disk so no call was + repeated. The harness should assume the same 60/h constraint for manual + runs. +- API license field is unreliable and must be verified against raw LICENSE + files: community.postgresql, ansible.posix, and kubernetes.core report + NOASSERTION but ship GPL-3.0 `COPYING`/`LICENSE` text; elastic/ansible- + elasticsearch reports NOASSERTION/"Other" but its LICENSE is Apache-2.0. + All license claims in this file state their source for this reason. +- Moved and dead repositories: nginxinc/ansible-role-nginx returns 301 + (moved to nginx/ansible-role-nginx); atosatto/ansible-role-elasticsearch + returns 404. Repos can move between screening and harness execution — the + manifest should store the resolved owner/name plus SHA, and the runner + should fail loudly on redirects rather than follow them silently. +- Unverifiable candidate names from the original request: mongo_single could + not be matched to any well-known, licensed Ansible role (GitHub name + search returns unrelated JavaScript coursework); the geerlingguy + "docker_ce" collection does not exist under that name (only stale + docker-centos6/7/8-ansible playbook repos). Both were rejected rather + than guessed at. +- Grep-based marker counts have known error modes observed in this corpus: + `with_cves` matched a filename (`fs_with_cves.sh`) and `with_efi` matched + a variable (`booted_with_efi`) in UBUNTU22-CIS / openstack/ansible- + hardening, inflating naive `with_` counts by 5 lines total. Counts are + line counts, not occurrence counts: multiline `when` conditions and + folded tasks are counted once per line, and nameless tasks (common in + legacy-style roles) are excluded from `- name:` task counts, so task + totals are lower bounds. The harness must parse YAML rather than grep; + grep numbers in this file are screening estimates, good enough for + hypotheses but not for exact assertions. +- Category gap: no shortlisted repo uses `rescue:` or `always:` (0 + occurrences across all 296 fetched task files), despite block/rescue/ + always being a target marker. The shortlist exercises plain blocks (189 + in CIS, 38 in the nginx role) but not error-handling flows. A future + corpus revision should add a block/rescue/always-heavy role; none was + found among the recommended families during this screening round. +- The legacy `collections:` keyword is absent from all shortlisted repos + (0 occurrences), so the corpus cannot assert anything about it. +- elastic/ansible-elasticsearch is archived (last push 2022-06-24). Kept + deliberately for its unique legacy syntax (bare `include:`, zero FQCN), + accepting that it will never be updated; no maintained replacement with + the same syntax profile was identified during screening. +- ansible-lockdown/UBUNTU22-CIS defaults to branch `devel`, not main/ + master. Screening tooling that assumes main/master would misresolve it; + pin by SHA everywhere. Lockdown licenses also vary per benchmark repo + over time (UBUNTU22-CIS is MIT today; other lockdown repos historically + GPL-3.0) — verify per repo when extending the corpus. +- Destructive automation concern: hardening roles (CIS, devsec, openstack) + contain destructive remediation tasks (auditd, PAM, SSH). The harness is + static-analysis-only and must never execute corpus playbooks, which + contains this risk; no embedded credentials were observed in the fetched + task files, but no dedicated secret scan was performed and the harness + should not assume it. +- All 11 git trees returned `truncated: false` (largest: 984 entries for + prometheus-community/ansible), so tree-derived file counts are complete. + Repos exceeding the tree API size/entry limits would truncate; not hit + during this research, but the harness should assert on the `truncated` + field rather than assume completeness. + +## Research log + +Date: 2026-09-12. All calls made unauthenticated from this workstation; +responses cached under the session temp directory during research. + +GitHub REST API (core quota, limit 60/h): + +- `GET /rate_limit` x5 (free; does not consume quota) — final reading + `used=42 remaining=18`. +- `GET /repos/{owner}/{repo}` x19: geerlingguy/ansible-role-{docker,nginx, + mysql,postgresql,jenkins}, ansible-collections/{community.general, + community.docker, community.postgresql, ansible.posix, kubernetes.core}, + prometheus-community/ansible, dev-sec/ansible-collection-hardening, + elastic/ansible-elasticsearch, ansible-lockdown/UBUNTU22-CIS, + openstack/ansible-hardening, nginxinc/ansible-role-nginx (301), + atosatto/ansible-role-elasticsearch (404), nginx/ansible-role-nginx. +- `GET /repos/{owner}/{repo}/commits/{branch}` x11 (head SHAs for the ten + shortlist repos plus the nginx role). +- `GET /repos/{owner}/{repo}/git/trees/{sha}?recursive=1` x11 (all + `truncated: false`). +- `GET /repositories/117019566` x1 (resolved the nginxinc 301 redirect to + nginx/ansible-role-nginx). + +GitHub search API (separate quota): `GET /search/repositories` x2 +(`mongo_single in:name`; `docker_ce user:geerlingguy`). + +Raw fetches (raw.githubusercontent.com, not rate limited): 296 task/handler +YAML files pinned at the SHAs in the candidate table (docker 7, geerlingguy +nginx 10, mysql 11, postgresql 11, nginx role 32, prometheus 91, devsec 41, +UBUNTU22-CIS 69, elastic 21, openstack 21), plus 5 LICENSE/COPYING +verifications and 5 defaults-file fetches. Zero raw fetch failures. + +Failures and interventions: two dead/moved repos (301 nginxinc, 404 +atosatto) handled by redirect resolution and rejection; one local tooling +error (xargs invocation too long during the raw-fetch loop, fixed by +switching to a two-column URL/destination pairs file); no HTTP 403/429 and +no tree truncation at any point. diff --git a/tests/diagrams/test_architecture_diagram.py b/tests/diagrams/test_architecture_diagram.py index 6d351e3..53912d9 100644 --- a/tests/diagrams/test_architecture_diagram.py +++ b/tests/diagrams/test_architecture_diagram.py @@ -125,8 +125,8 @@ def test_generate_with_multiple_task_files(self): assert "5 tasks" in diagram assert "10 tasks" in diagram assert "3 tasks" in diagram - # Should show flow from first to last - assert "tasks_install_yml --> tasks_validate_yml" in diagram + # No include data: no inter-file flow edges should be fabricated + assert "tasks_install_yml --> tasks_validate_yml" not in diagram def test_generate_with_handlers(self): """Test diagram with handlers.""" @@ -236,7 +236,14 @@ def test_generate_complex_role_full_diagram(self): { "file": "install.yml", "tasks": [ - {"name": f"Task {i}", "module": "package"} for i in range(12) + {"name": f"Task {i}", "module": "package"} for i in range(11) + ] + + [ + { + "name": "Include configure", + "module": "include_tasks", + "include_target": "configure.yml", + } ], }, { @@ -292,10 +299,71 @@ def test_generate_complex_role_full_diagram(self): # Check data flow (variables flow to first task file) assert "defaults --> tasks_install_yml" in diagram assert "vars --> tasks_install_yml" in diagram - assert "tasks_install_yml --> tasks_configure_yml" in diagram + # Statically resolvable include becomes a labeled edge + assert 'tasks_install_yml -."includes".-> tasks_configure_yml' in diagram assert "notify" in diagram assert "tasks_configure_yml --> external" in diagram + def test_generate_with_include_edges(self): + """Include/import targets become edges; templated targets are skipped.""" + role_info = { + "name": "test_role", + "defaults": [], + "vars": [], + "tasks": [ + { + "file": "main.yml", + "tasks": [ + {"name": "Load OS vars", "module": "include_vars"}, + { + "name": "Unnamed", + "module": "include_tasks", + "include_target": "{{ ansible_facts['os_family'] }}.yml", + }, + { + "name": "Unnamed", + "module": "import_tasks", + "include_target": "setup.yml", + }, + { + "name": "Unnamed", + "module": "include", + "include_target": "extra.yml", + }, + ], + }, + {"file": "setup.yml", "tasks": [{"name": "Setup"}]}, + {"file": "subdir/extra.yml", "tasks": [{"name": "Extra"}]}, + {"file": "orphan.yml", "tasks": [{"name": "Orphan"}]}, + ], + "handlers": [], + } + + complexity_report = ComplexityReport( + metrics=ComplexityMetrics( + total_tasks=7, + task_files=4, + handlers=0, + conditional_tasks=0, + max_tasks_per_file=4, + avg_tasks_per_file=1.75, + ), + category=ComplexityCategory.SIMPLE, + integration_points=[], + ) + + diagram = generate_component_architecture(role_info, complexity_report) + + assert diagram is not None + # Exact-match target + assert 'tasks_main_yml -."includes".-> tasks_setup_yml' in diagram + # Basename match against nested file + assert 'tasks_main_yml -."includes".-> tasks_subdir_extra_yml' in diagram + # Templated target must not be drawn as an edge + assert "os_family" not in diagram + # No fabricated edges to unrelated files + assert "--> tasks_orphan_yml" not in diagram + def test_generate_with_no_role_info(self): """Test that None is returned when role_info is None.""" diagram = generate_component_architecture(None, None) diff --git a/tests/end_to_end/test_json_output_clean.py b/tests/end_to_end/test_json_output_clean.py new file mode 100644 index 0000000..fd500fd --- /dev/null +++ b/tests/end_to_end/test_json_output_clean.py @@ -0,0 +1,65 @@ +"""End-to-end tests for machine-readable JSON output cleanliness. + +When ``--output-format json`` is used, stdout must carry only valid JSON. +Logging is routed to stderr so the output can be piped or parsed directly. +""" + +import json + +from click.testing import CliRunner + +from docsible.cli import cli + + +def _make_role(tmp_path): + role = tmp_path / "test_role" + (role / "tasks").mkdir(parents=True) + (role / "tasks" / "main.yml").write_text( + "---\n- name: Say hello\n debug:\n msg: hello\n" + ) + return role + + +def test_analyze_role_json_stdout_is_pure_json(tmp_path): + role = _make_role(tmp_path) + runner = CliRunner() + result = runner.invoke( + cli, ["analyze", "role", "--role", str(role), "--output-format", "json"] + ) + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["role"] == "test_role" + assert "findings" in payload + assert "summary" in payload + assert "truncated" in payload + + +def test_analyze_role_json_stdout_clean_even_with_verbose(tmp_path): + """Verbose forces DEBUG logging; stdout must still parse as pure JSON.""" + role = _make_role(tmp_path) + runner = CliRunner() + result = runner.invoke( + cli, + [ + "--verbose", + "analyze", + "role", + "--role", + str(role), + "--output-format", + "json", + ], + ) + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["role"] == "test_role" + + +def test_validate_role_json_stdout_is_pure_json(tmp_path): + role = _make_role(tmp_path) + runner = CliRunner() + result = runner.invoke( + cli, ["validate", "role", "--role", str(role), "--output-format", "json"] + ) + assert result.exit_code == 0 + json.loads(result.stdout) diff --git a/tests/renderers/test_markdown_processor.py b/tests/renderers/test_markdown_processor.py new file mode 100644 index 0000000..f846270 --- /dev/null +++ b/tests/renderers/test_markdown_processor.py @@ -0,0 +1,42 @@ +"""Tests for MarkdownProcessor blank-line normalization behavior. + +Excessive consecutive blank lines are always collapsed to the validator's +maximum, even without --auto-fix, so generated output complies with +docsible's own markdown validation by default. +""" + +from docsible.renderers.processors.markdown_processor import MarkdownProcessor + + +def test_collapses_excessive_blank_lines_without_auto_fix(): + processor = MarkdownProcessor(validate=False, auto_fix=False) + markdown = "# Title\n\n\n\n\nBody\n" + assert processor.process(markdown) == "# Title\n\n\nBody\n" + + +def test_keeps_allowed_blank_lines(): + processor = MarkdownProcessor(validate=False, auto_fix=False) + markdown = "# Title\n\n\nBody\n" + assert processor.process(markdown) == markdown + + +def test_auto_fix_still_applies_other_fixes(): + processor = MarkdownProcessor(validate=False, auto_fix=True) + markdown = "Line with trailing \n\tTabbed line\n\n\n\n\nEnd\n" + result = processor.process(markdown) + assert "Line with trailing\n" in result + assert "\t" not in result + assert "\n\n\n\n" not in result + + +def test_strict_validation_raises_on_errors(): + processor = MarkdownProcessor( + validate=True, auto_fix=False, strict_validation=True + ) + markdown = "# Title\n\n```yaml\nunclosed\n" + try: + processor.process(markdown) + except ValueError as exc: + assert "Markdown validation failed" in str(exc) + else: + raise AssertionError("expected ValueError from strict validation") diff --git a/tests/utils/test_special_tasks_keys.py b/tests/utils/test_special_tasks_keys.py new file mode 100644 index 0000000..f4380d7 --- /dev/null +++ b/tests/utils/test_special_tasks_keys.py @@ -0,0 +1,87 @@ +"""Tests for task key extraction: blocks and include/import targets.""" + +from docsible.utils.special_tasks_keys import process_special_task_keys + + +def test_regular_task_has_no_include_target(): + result = process_special_task_keys({"name": "Install", "apt": {"name": "nginx"}}) + assert result == [ + {"name": "Install", "module": "apt", "type": "task", "when": None} + ] + assert "include_target" not in result[0] + + +def test_include_tasks_static_target_captured(): + result = process_special_task_keys({"include_tasks": "setup.yml"}) + assert result[0]["module"] == "include_tasks" + assert result[0]["include_target"] == "setup.yml" + + +def test_fqcn_include_tasks_target_captured(): + result = process_special_task_keys( + {"ansible.builtin.include_tasks": "configure.yml"} + ) + assert result[0]["module"] == "ansible.builtin.include_tasks" + assert result[0]["include_target"] == "configure.yml" + + +def test_fqcn_import_tasks_target_captured(): + result = process_special_task_keys({"ansible.builtin.import_tasks": "users.yml"}) + assert result[0]["module"] == "ansible.builtin.import_tasks" + assert result[0]["include_target"] == "users.yml" + + +def test_legacy_bare_include_target_captured(): + result = process_special_task_keys({"include": "legacy.yml"}) + assert result[0]["module"] == "include" + assert result[0]["include_target"] == "legacy.yml" + + +def test_include_dict_file_form_captured(): + result = process_special_task_keys( + {"include_tasks": {"file": "nested.yml", "apply": {"tags": ["x"]}}} + ) + assert result[0]["include_target"] == "nested.yml" + + +def test_templated_include_target_captured_as_is(): + result = process_special_task_keys( + {"include_tasks": "{{ ansible_facts['os_family'] }}.yml"} + ) + assert result[0]["include_target"] == "{{ ansible_facts['os_family'] }}.yml" + + +def test_include_target_pipes_escaped(): + result = process_special_task_keys({"include_tasks": "{{ item | upper }}.yml"}) + assert "|" not in result[0]["include_target"] + assert "¦" in result[0]["include_target"] + + +def test_list_include_target_not_captured(): + result = process_special_task_keys({"include_tasks": ["a.yml", "b.yml"]}) + assert result[0]["module"] == "include_tasks" + assert "include_target" not in result[0] + + +def test_include_vars_target_not_captured(): + result = process_special_task_keys({"include_vars": "Debian.yml"}) + assert result[0]["module"] == "include_vars" + assert "include_target" not in result[0] + + +def test_include_role_target_captured(): + result = process_special_task_keys({"include_role": "common"}) + assert result[0]["module"] == "include_role" + assert result[0]["include_target"] == "common" + + +def test_block_task_shape_unchanged(): + result = process_special_task_keys( + {"name": "Handle failure", "block": [{"debug": {"msg": "try"}}], "rescue": []} + ) + assert result[0]["module"] == "block" + assert result[0]["type"] == "block" + assert "include_target" not in result[0] + assert result[1]["module"] == "debug" + # An empty rescue list contributes no rescue entry + assert len(result) == 2 From 9e3b02500cd10b20ec6d790f253819a16de6508e Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 00:50:16 +0200 Subject: [PATCH 02/20] removing confusing license output and confusing terminal output --- .../document_role/orchestrators/role_orchestrator.py | 8 ++++++-- docsible/templates/role/sections/galaxy_info.jinja2 | 4 ++-- docsible/utils/template/filters.py | 9 +++++++++ docsible/utils/yaml/loader.py | 4 +++- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index 3104431..d209e4d 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -155,7 +155,9 @@ def execute(self) -> None: return # Step 9: Render documentation - self._render_documentation(role_info, role_path, analysis_report, diagrams, dependency_data) + self._render_documentation( + role_info, role_path, analysis_report, diagrams, dependency_data, recommendations + ) def _validate_paths(self) -> Path: """Validate and return role path. @@ -327,6 +329,7 @@ def _display_dry_run( analysis_report, diagrams: dict, dependency_data: dict, + recommendations: list[Recommendation], ) -> None: """Display dry-run summary. @@ -336,6 +339,7 @@ def _display_dry_run( analysis_report: Complexity analysis report diagrams: Generated diagrams dictionary dependency_data: Dependency matrix data + recommendations: Findings to reflect in the success summary """ flags = { "generate_graph": self.context.diagrams.generate_graph, @@ -518,7 +522,7 @@ def _render_documentation( success_msg = formatter.format_success( output_file=readme_path, complexity=analysis_report, - recommendations=[], # recommendations already shown separately above + recommendations=recommendations, ) click.echo("\n" + success_msg) else: diff --git a/docsible/templates/role/sections/galaxy_info.jinja2 b/docsible/templates/role/sections/galaxy_info.jinja2 index ea40d1b..3bfae1a 100644 --- a/docsible/templates/role/sections/galaxy_info.jinja2 +++ b/docsible/templates/role/sections/galaxy_info.jinja2 @@ -1,7 +1,7 @@ {% if role.meta and role.meta.galaxy_info %} ### Author Information - **Author**: {{ role.meta.galaxy_info.author or 'Unknown' }} -- **License**: {{ role.meta.galaxy_info.license or 'Not specified' }} +- **License**: {{ role.meta.galaxy_info.license | normalize_license or 'Not specified' }} - **Platforms**: {% for platform in role.meta.galaxy_info.platforms %} - {{ platform.name }}: {{ platform.versions }} @@ -58,4 +58,4 @@ Description: {{ role.meta.galaxy_info.description or 'Not available.' }} {%- if role.docsible.critical %} | Critical ⚠️ | {{ role.docsible.critical | escape_table_cell }} | {%- endif %} -{% endif %} \ No newline at end of file +{% endif %} diff --git a/docsible/utils/template/filters.py b/docsible/utils/template/filters.py index 010c618..b3e8545 100644 --- a/docsible/utils/template/filters.py +++ b/docsible/utils/template/filters.py @@ -10,6 +10,14 @@ logger = logging.getLogger(__name__) +def normalize_license(value: Any) -> str: + """Remove redundant ``license (...)`` wrapping from Galaxy metadata.""" + text = str(value or "").strip() + if text.lower().startswith("license (") and text.endswith(")"): + return text[9:-1].strip() + return text + + def escape_table_cell( value: Any, max_length: int | None = None, truncate_indicator: str = "..." ) -> str: @@ -170,4 +178,5 @@ def safe_join(items: Any, separator: str = ", ", max_items: int | None = None) - "escape_table_cell": escape_table_cell, "escape_table_value": escape_table_value, "safe_join": safe_join, + "normalize_license": normalize_license, } diff --git a/docsible/utils/yaml/loader.py b/docsible/utils/yaml/loader.py index 4a78476..ccef1d3 100644 --- a/docsible/utils/yaml/loader.py +++ b/docsible/utils/yaml/loader.py @@ -356,7 +356,9 @@ def _format_value_for_display(value: Any, multiline_indicator: str | None) -> An Formatted value """ if multiline_indicator: - return f"" + # Markdown table cells cannot preserve YAML block formatting. Keep the + # source value readable rather than replacing it with a placeholder. + return " ".join(value.split()) if isinstance(value, str) else value elif isinstance(value, list): return [] elif isinstance(value, dict): From 1c9978880827587e3f0bb17893f5f180687061df Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 01:03:29 +0200 Subject: [PATCH 03/20] work in progress for role graph renderer base to be consumed by mermaid and later other renderer --- docsible/commands/document_role/helpers.py | 6 +- .../orchestrators/role_orchestrator.py | 13 +- docsible/diagrams/types/architecture.py | 34 +- docsible/graphs/__init__.py | 21 ++ docsible/graphs/role_execution.py | 302 ++++++++++++++++++ docsible/renderers/readme_renderer.py | 2 + .../role/sections/adaptive_diagrams.jinja2 | 11 +- tests/graphs/test_role_execution.py | 48 +++ 8 files changed, 425 insertions(+), 12 deletions(-) create mode 100644 docsible/graphs/__init__.py create mode 100644 docsible/graphs/role_execution.py create mode 100644 tests/graphs/test_role_execution.py diff --git a/docsible/commands/document_role/helpers.py b/docsible/commands/document_role/helpers.py index ba5ec4e..45c1fff 100644 --- a/docsible/commands/document_role/helpers.py +++ b/docsible/commands/document_role/helpers.py @@ -251,7 +251,10 @@ def generate_mermaid_diagrams( def generate_integration_and_architecture_diagrams( - generate_graph: bool, role_info: dict[str, Any], analysis_report: Any + generate_graph: bool, + role_info: dict[str, Any], + analysis_report: Any, + execution_graph: Any | None = None, ) -> tuple[str | None, str | None]: """Generate integration boundary and component architecture diagrams. @@ -297,6 +300,7 @@ def generate_integration_and_architecture_diagrams( architecture_diagram = generate_component_architecture( role_info=role_info, complexity_report=analysis_report, + execution_graph=execution_graph, ) logger.info( f"Generated component architecture diagram for {analysis_report.category.value.upper()} role" diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index d209e4d..0529973 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -272,6 +272,9 @@ def _generate_diagrams( generate_integration_and_architecture_diagrams, generate_mermaid_diagrams, ) + from docsible.graphs import build_role_execution_graph + + execution_graph = build_role_execution_graph(role_info) # Generate task diagrams diagrams = generate_mermaid_diagrams( @@ -285,12 +288,15 @@ def _generate_diagrams( # Add generate_graph flag for formatter diagrams["generate_graph"] = self.context.diagrams.generate_graph + diagrams["execution_graph"] = execution_graph + diagrams["execution_phases"] = execution_graph.execution_phases() # Generate integration and architecture diagrams integration_boundary, architecture = generate_integration_and_architecture_diagrams( generate_graph=self.context.diagrams.generate_graph, role_info=role_info, analysis_report=analysis_report, + execution_graph=execution_graph, ) diagrams["integration_boundary_diagram"] = integration_boundary @@ -329,7 +335,6 @@ def _display_dry_run( analysis_report, diagrams: dict, dependency_data: dict, - recommendations: list[Recommendation], ) -> None: """Display dry-run summary. @@ -339,7 +344,6 @@ def _display_dry_run( analysis_report: Complexity analysis report diagrams: Generated diagrams dictionary dependency_data: Dependency matrix data - recommendations: Findings to reflect in the success summary """ flags = { "generate_graph": self.context.diagrams.generate_graph, @@ -453,6 +457,7 @@ def _render_options( "state_diagram": diagrams.get("state_diagram"), "integration_boundary_diagram": diagrams.get("integration_boundary_diagram"), "architecture_diagram": diagrams.get("architecture_diagram"), + "execution_phases": diagrams.get("execution_phases"), "complexity_report": analysis_report, "include_complexity": include_complexity, "dependency_matrix": dependency_data["dependency_matrix"], @@ -474,6 +479,7 @@ def _render_documentation( analysis_report, diagrams: dict, dependency_data: dict, + recommendations: list[Recommendation] | None = None, ) -> None: """Render final documentation. @@ -483,6 +489,7 @@ def _render_documentation( analysis_report: Complexity analysis report diagrams: Generated diagrams dictionary dependency_data: Dependency matrix data + recommendations: Findings to reflect in the success summary """ from docsible.renderers.readme_renderer import ReadmeRenderer from docsible.renderers.tag_manager import manage_docsible_file_keys @@ -522,7 +529,7 @@ def _render_documentation( success_msg = formatter.format_success( output_file=readme_path, complexity=analysis_report, - recommendations=recommendations, + recommendations=recommendations or [], ) click.echo("\n" + success_msg) else: diff --git a/docsible/diagrams/types/architecture.py b/docsible/diagrams/types/architecture.py index f84b831..f6e6af3 100644 --- a/docsible/diagrams/types/architecture.py +++ b/docsible/diagrams/types/architecture.py @@ -11,7 +11,7 @@ def generate_component_architecture( - role_info: dict[str, Any] | None, complexity_report: Any + role_info: dict[str, Any] | None, complexity_report: Any, execution_graph: Any | None = None ) -> str | None: """ Generate component architecture diagram for complex roles. @@ -104,8 +104,22 @@ def generate_component_architecture( # Data flow connections lines.append(" %% Data Flow") - # Variables flow to tasks - if has_variables and task_files: + # Variables flow only to files with source-backed variable references. + if execution_graph is not None: + variable_edges: set[tuple[str, str]] = set() + for edge in execution_graph.edges: + if edge.kind.value != "uses_variable" or edge.target_id is None: + continue + task = execution_graph.nodes.get(edge.source_id) + variable = execution_graph.nodes.get(edge.target_id) + if not task or not variable: + continue + variable_node = "defaults" if variable.metadata.get("scope") == "defaults" else "vars" + task_id = f"tasks_{task.metadata['file'].replace('.', '_').replace('/', '_')}" + variable_edges.add((variable_node, task_id)) + for variable_node, task_id in sorted(variable_edges): + lines.append(f" {variable_node} --> {task_id}") + elif has_variables and task_files: first_task_id = ( f"tasks_{task_files[0].get('file', 'file0').replace('.', '_').replace('/', '_')}" ) @@ -146,8 +160,18 @@ def generate_component_architecture( lines.append(f' {source_id} -."includes".-> {target_id}') added_include_edges.add((source_id, target_id)) - # Tasks to handlers (notification) - if task_files and handlers_count > 0: + # Tasks to handlers use source-backed notify relationships when available. + if execution_graph is not None: + notifying_files: set[str] = set() + for edge in execution_graph.edges: + if edge.kind.value == "notifies_handler" and edge.target_id: + task = execution_graph.nodes.get(edge.source_id) + if task: + notifying_files.add(task.metadata["file"]) + for file_name in sorted(notifying_files): + task_id = f"tasks_{file_name.replace('.', '_').replace('/', '_')}" + lines.append(f' {task_id} -."notify".-> handlers') + elif task_files and handlers_count > 0: last_task_id = ( f"tasks_{task_files[-1].get('file', 'fileN').replace('.', '_').replace('/', '_')}" ) diff --git a/docsible/graphs/__init__.py b/docsible/graphs/__init__.py new file mode 100644 index 0000000..5da3a85 --- /dev/null +++ b/docsible/graphs/__init__.py @@ -0,0 +1,21 @@ +"""Source-backed execution and relationship graphs for Ansible roles.""" + +from docsible.graphs.role_execution import ( + EdgeKind, + GraphEdge, + GraphNode, + NodeKind, + ResolutionStatus, + RoleExecutionGraph, + build_role_execution_graph, +) + +__all__ = [ + "EdgeKind", + "GraphEdge", + "GraphNode", + "NodeKind", + "ResolutionStatus", + "RoleExecutionGraph", + "build_role_execution_graph", +] diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py new file mode 100644 index 0000000..e32183b --- /dev/null +++ b/docsible/graphs/role_execution.py @@ -0,0 +1,302 @@ +"""Build a renderer-independent execution graph from loaded Ansible role facts.""" + +from __future__ import annotations + +import re +from collections import deque +from dataclasses import asdict, dataclass, field +from enum import Enum +from pathlib import PurePosixPath +from typing import Any + + +class NodeKind(str, Enum): + ROLE = "role" + TASK_FILE = "task_file" + TASK = "task" + HANDLER = "handler" + VARIABLE = "variable" + EXTERNAL_ROLE = "external_role" + + +class EdgeKind(str, Enum): + CONTAINS = "contains" + INCLUDES_TASK_FILE = "includes_task_file" + IMPORTS_TASK_FILE = "imports_task_file" + INCLUDES_ROLE = "includes_role" + IMPORTS_ROLE = "imports_role" + NOTIFIES_HANDLER = "notifies_handler" + USES_VARIABLE = "uses_variable" + + +class ResolutionStatus(str, Enum): + STATIC = "static" + DYNAMIC = "dynamic" + UNKNOWN = "unknown" + UNRESOLVED_EXTERNAL = "unresolved_external" + + +@dataclass(frozen=True) +class SourceLocation: + file: str + line: int | None = None + + +@dataclass(frozen=True) +class GraphNode: + id: str + kind: NodeKind + label: str + source: SourceLocation | None = None + metadata: dict[str, Any] = field(default_factory=dict) + + +@dataclass(frozen=True) +class GraphEdge: + kind: EdgeKind + source_id: str + target_id: str | None + resolution: ResolutionStatus + source: SourceLocation + condition: str | None = None + loop: str | None = None + target_expression: str | None = None + metadata: dict[str, Any] = field(default_factory=dict) + + +@dataclass +class RoleExecutionGraph: + """The small interface shared by phase and renderer projections.""" + + role_id: str + nodes: dict[str, GraphNode] = field(default_factory=dict) + edges: list[GraphEdge] = field(default_factory=list) + + def add_node(self, node: GraphNode) -> None: + self.nodes.setdefault(node.id, node) + + def add_edge(self, edge: GraphEdge) -> None: + if edge not in self.edges: + self.edges.append(edge) + + def execution_phases(self) -> list[dict[str, Any]]: + """Return deterministic static traversal phases plus unreachable files.""" + files = [node for node in self.nodes.values() if node.kind is NodeKind.TASK_FILE] + file_ids = {node.id for node in files} + roots = sorted(node.id for node in files if node.metadata.get("file") == "main.yml") + if not roots: + roots = sorted(file_ids) + + task_files_by_task: dict[str, str] = { + edge.target_id: edge.source_id + for edge in self.edges + if edge.kind is EdgeKind.CONTAINS + and edge.source_id in file_ids + and edge.target_id is not None + } + outgoing: dict[str, list[GraphEdge]] = {} + for edge in self.edges: + if ( + edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE} + and edge.resolution is ResolutionStatus.STATIC + and edge.target_id in file_ids + ): + source_file_id = task_files_by_task.get(edge.source_id) + if source_file_id is not None: + outgoing.setdefault(source_file_id, []).append(edge) + + phases: list[dict[str, Any]] = [] + seen: set[str] = set() + queue: deque[tuple[str, str, list[GraphEdge]]] = deque( + (root, "entrypoint", []) for root in roots + ) + while queue: + node_id, phase_kind, incoming = queue.popleft() + if node_id in seen: + continue + seen.add(node_id) + node = self.nodes[node_id] + phases.append( + { + "file": node.metadata["file"], + "task_count": node.metadata["task_count"], + "kind": phase_kind, + "conditions": sorted({edge.condition for edge in incoming if edge.condition}), + } + ) + for edge in outgoing.get(node_id, []): + if edge.target_id is not None: + queue.append( + (edge.target_id, "conditional" if edge.condition else "static", [edge]) + ) + + for node in sorted(files, key=lambda item: item.metadata["file"]): + if node.id not in seen: + phases.append( + { + "file": node.metadata["file"], + "task_count": node.metadata["task_count"], + "kind": "unreachable", + "conditions": [], + } + ) + return phases + + def to_dict(self) -> dict[str, Any]: + return { + "role_id": self.role_id, + "nodes": [asdict(node) for node in self.nodes.values()], + "edges": [asdict(edge) for edge in self.edges], + } + + +_TASK_FILE_ACTIONS = {"include", "include_tasks", "import_tasks"} +_ROLE_ACTIONS = {"include_role", "import_role"} +_VARIABLE_PATTERN = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\b") + + +def build_role_execution_graph(role_info: dict[str, Any]) -> RoleExecutionGraph: + """Build source-backed facts without making renderer-specific guesses.""" + role_name = str(role_info.get("name", "unknown")) + graph = RoleExecutionGraph(role_id=f"role:{role_name}") + graph.add_node(GraphNode(graph.role_id, NodeKind.ROLE, role_name)) + + task_files = role_info.get("tasks", []) + file_ids = { + item.get("file", "unknown"): f"task_file:{role_name}:{item.get('file', 'unknown')}" + for item in task_files + } + for item in task_files: + file_name = item.get("file", "unknown") + file_id = file_ids[file_name] + graph.add_node( + GraphNode( + file_id, + NodeKind.TASK_FILE, + file_name, + SourceLocation(file_name), + {"file": file_name, "task_count": len(item.get("tasks", []))}, + ) + ) + graph.add_edge( + GraphEdge(EdgeKind.CONTAINS, graph.role_id, file_id, ResolutionStatus.STATIC, SourceLocation(file_name)) + ) + + variables = _add_variables(graph, role_name, role_info) + handlers = _add_handlers(graph, role_name, role_info) + for item in task_files: + _add_tasks(graph, role_name, item, file_ids, variables, handlers) + return graph + + +def _add_variables(graph: RoleExecutionGraph, role_name: str, role_info: dict[str, Any]) -> dict[str, str]: + variables: dict[str, str] = {} + for scope in ("defaults", "vars"): + for data_file in role_info.get(scope, []): + for key, details in data_file.get("data", {}).items(): + name = key.split(".", 1)[0] + node_id = f"variable:{role_name}:{scope}:{name}" + variables.setdefault(name, node_id) + graph.add_node(GraphNode(node_id, NodeKind.VARIABLE, name, SourceLocation(data_file["file"], details.get("line")), {"scope": scope})) + return variables + + +def _add_handlers(graph: RoleExecutionGraph, role_name: str, role_info: dict[str, Any]) -> dict[str, str]: + handlers: dict[str, str] = {} + for handler in role_info.get("handlers", []): + node_id = f"handler:{role_name}:{handler['name']}" + graph.add_node(GraphNode(node_id, NodeKind.HANDLER, handler["name"], SourceLocation(f"handlers/{handler.get('file', 'main.yml')}"))) + for name in [handler["name"], *handler.get("listen", [])]: + handlers[name] = node_id + return handlers + + +def _add_tasks(graph: RoleExecutionGraph, role_name: str, task_file: dict[str, Any], file_ids: dict[str, str], variables: dict[str, str], handlers: dict[str, str]) -> None: + file_name = task_file.get("file", "unknown") + raw_tasks = task_file.get("mermaid", []) + line_ranges = task_file.get("line_ranges", []) + for index, task in _walk_tasks(raw_tasks): + line = line_ranges[index[0]][0] if index and index[0] < len(line_ranges) else None + source = SourceLocation(f"tasks/{file_name}", line) + task_id = f"task:{role_name}:{file_name}:{'.'.join(map(str, index))}" + graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, {"file": file_name, "module": _module_name(task)})) + graph.add_edge(GraphEdge(EdgeKind.CONTAINS, file_ids[file_name], task_id, ResolutionStatus.STATIC, source)) + _add_variable_edges(graph, task_id, task, variables, source) + _add_notification_edges(graph, task_id, task, handlers, source) + _add_composition_edge(graph, task_id, task, file_name, file_ids, source) + + +def _walk_tasks(tasks: list[Any], prefix: tuple[int, ...] = ()): + for index, task in enumerate(tasks): + if not isinstance(task, dict): + continue + path = (*prefix, index) + yield path, task + for section in ("block", "rescue", "always"): + nested = task.get(section) + if isinstance(nested, list): + yield from _walk_tasks(nested, path) + + +def _module_name(task: dict[str, Any]) -> str: + for key in task: + if key.split(".")[-1] in _TASK_FILE_ACTIONS | _ROLE_ACTIONS: + return key + excluded = {"name", "when", "notify", "tags", "register", "loop", "loop_control", "block", "rescue", "always", "vars"} + return next((key for key in task if key not in excluded), "unknown") + + +def _add_variable_edges(graph: RoleExecutionGraph, task_id: str, task: dict[str, Any], variables: dict[str, str], source: SourceLocation) -> None: + text = str(task) + for name, node_id in variables.items(): + if re.search(rf"\b{re.escape(name)}\b", text): + graph.add_edge(GraphEdge(EdgeKind.USES_VARIABLE, task_id, node_id, ResolutionStatus.STATIC, source)) + + +def _add_notification_edges(graph: RoleExecutionGraph, task_id: str, task: dict[str, Any], handlers: dict[str, str], source: SourceLocation) -> None: + notified = task.get("notify", []) + for name in [notified] if isinstance(notified, str) else notified if isinstance(notified, list) else []: + graph.add_edge(GraphEdge(EdgeKind.NOTIFIES_HANDLER, task_id, handlers.get(name), ResolutionStatus.STATIC if name in handlers else ResolutionStatus.UNKNOWN, source, target_expression=name)) + + +def _add_composition_edge(graph: RoleExecutionGraph, task_id: str, task: dict[str, Any], source_file: str, file_ids: dict[str, str], source: SourceLocation) -> None: + module = _module_name(task) + short_module = module.split(".")[-1] + if short_module not in _TASK_FILE_ACTIONS | _ROLE_ACTIONS: + return + raw_target = task.get(module) + if isinstance(raw_target, dict): + raw_target = raw_target.get("file" if short_module in _TASK_FILE_ACTIONS else "name") + target = str(raw_target) if raw_target is not None else "" + dynamic = "{{" in target or "{%" in target + kind = EdgeKind.IMPORTS_TASK_FILE if short_module == "import_tasks" else EdgeKind.INCLUDES_TASK_FILE + if short_module in _ROLE_ACTIONS: + kind = EdgeKind.IMPORTS_ROLE if short_module == "import_role" else EdgeKind.INCLUDES_ROLE + target_id = f"external_role:{target}" if target and not dynamic else None + if target_id: + graph.add_node(GraphNode(target_id, NodeKind.EXTERNAL_ROLE, target, metadata={"role": target})) + resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.UNRESOLVED_EXTERNAL + else: + target_id = _resolve_task_file(target, source_file, file_ids) if not dynamic else None + resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.STATIC if target_id else ResolutionStatus.UNKNOWN + graph.add_edge(GraphEdge(kind, task_id, target_id, resolution, source, condition=_condition(task), loop=_loop(task), target_expression=target or None)) + + +def _resolve_task_file(target: str, source_file: str, file_ids: dict[str, str]) -> str | None: + candidates = [target.removeprefix("tasks/"), str(PurePosixPath(source_file).parent / target)] + for candidate in candidates: + if candidate in file_ids: + return file_ids[candidate] + matches = [node_id for file_name, node_id in file_ids.items() if PurePosixPath(file_name).name == PurePosixPath(target).name] + return matches[0] if len(matches) == 1 else None + + +def _condition(task: dict[str, Any]) -> str | None: + value = task.get("when") + return " and ".join(map(str, value)) if isinstance(value, list) else str(value) if value else None + + +def _loop(task: dict[str, Any]) -> str | None: + if "loop" in task: + return "loop" + return next((key for key in task if key.startswith("with_")), None) diff --git a/docsible/renderers/readme_renderer.py b/docsible/renderers/readme_renderer.py index 49fbfb9..0691420 100644 --- a/docsible/renderers/readme_renderer.py +++ b/docsible/renderers/readme_renderer.py @@ -74,6 +74,7 @@ def render_role( state_diagram: str | None = None, integration_boundary_diagram: str | None = None, architecture_diagram: str | None = None, + execution_phases: list[dict[str, Any]] | None = None, complexity_report: Any | None = None, include_complexity: bool | None = None, dependency_matrix: str | None = None, @@ -126,6 +127,7 @@ def render_role( state_diagram=state_diagram, integration_boundary_diagram=integration_boundary_diagram, architecture_diagram=architecture_diagram, + execution_phases=execution_phases, complexity_report=complexity_report, include_complexity=include_complexity, dependency_matrix=dependency_matrix, diff --git a/docsible/templates/role/sections/adaptive_diagrams.jinja2 b/docsible/templates/role/sections/adaptive_diagrams.jinja2 index 8164b9e..9a50d0b 100644 --- a/docsible/templates/role/sections/adaptive_diagrams.jinja2 +++ b/docsible/templates/role/sections/adaptive_diagrams.jinja2 @@ -89,13 +89,18 @@ This role integrates with {{ complexity_report.integration_points|length }} exte {% endif %} ### Execution Phases - -Due to complexity, this role is best understood through its execution phases: +These phases are a static traversal from the role entry point. Conditional +branches are annotated; dynamic includes remain unresolved. -{% for task_file_info in complexity_report.task_files_detail %} +{% for task_file_info in execution_phases or complexity_report.task_files_detail %} #### Phase {{ loop.index }}: {{ task_file_info.file }} **Tasks:** {{ task_file_info.task_count }} +{% if task_file_info.kind is defined and task_file_info.kind == 'conditional' %} +**Conditional path:** {{ task_file_info.conditions | join(' and ') }} +{% elif task_file_info.kind is defined and task_file_info.kind == 'unreachable' %} +**Static traversal:** not reached from the entry point +{% endif %} > **Note:** Document decision points, error handling, and key operations for this phase > diff --git a/tests/graphs/test_role_execution.py b/tests/graphs/test_role_execution.py new file mode 100644 index 0000000..73c1219 --- /dev/null +++ b/tests/graphs/test_role_execution.py @@ -0,0 +1,48 @@ +"""Tests for the source-backed role execution graph.""" + +from docsible.graphs import EdgeKind, ResolutionStatus, build_role_execution_graph + + +def test_builds_static_dynamic_and_notify_relationships(): + role_info = { + "name": "web", + "defaults": [{"file": "main.yml", "data": {"web_port": {"line": 1}}}], + "vars": [], + "handlers": [{"name": "restart web", "listen": ["restart"], "file": "main.yml"}], + "tasks": [ + { + "file": "main.yml", + "tasks": [{}, {}], + "line_ranges": [(1, 5), (6, 10)], + "mermaid": [ + { + "name": "Configure", + "template": {"src": "web.j2"}, + "notify": "restart", + "when": "web_port > 0", + }, + {"include_tasks": "setup.yml"}, + ], + }, + { + "file": "setup.yml", + "tasks": [{}], + "line_ranges": [(1, 3)], + "mermaid": [{"include_tasks": "{{ ansible_facts.os_family }}.yml"}], + }, + ], + } + + graph = build_role_execution_graph(role_info) + + assert any(edge.kind is EdgeKind.NOTIFIES_HANDLER and edge.target_id for edge in graph.edges) + static_include = next(edge for edge in graph.edges if edge.target_expression == "setup.yml") + assert static_include.kind is EdgeKind.INCLUDES_TASK_FILE + assert static_include.resolution is ResolutionStatus.STATIC + assert static_include.target_id == "task_file:web:setup.yml" + dynamic_include = next(edge for edge in graph.edges if edge.target_expression and "{{" in edge.target_expression) + assert dynamic_include.resolution is ResolutionStatus.DYNAMIC + assert dynamic_include.target_id is None + phases = graph.execution_phases() + assert [phase["file"] for phase in phases] == ["main.yml", "setup.yml"] + assert phases[1]["kind"] == "static" From f1d7616c5c679af909b8ad347a3aa412103ab7de Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 01:49:58 +0200 Subject: [PATCH 04/20] =?UTF-8?q?Isolated=20fixes=20-=20Terminal=20success?= =?UTF-8?q?=20message=20now=20receives=20real=20recommendations,=20so=20it?= =?UTF-8?q?=20no=20longer=20claims=20=E2=80=9Cno=20enhancement=20opportuni?= =?UTF-8?q?ties=E2=80=9D=20after=20warnings.=20-=20license=20(BSD,=20MIT)?= =?UTF-8?q?=20now=20renders=20as=20BSD,=20MIT.=20-=20Multiline=20YAML=20de?= =?UTF-8?q?faults=20now=20show=20normalized=20source=20content=20instead?= =?UTF-8?q?=20of=20=20placeholders.=20RoleExecut?= =?UTF-8?q?ionGraph=20-=20Added=20docsible/graphs/role=5Fexecution.py.=20-?= =?UTF-8?q?=20Public=20seam:=20build=5Frole=5Fexecution=5Fgraph(role=5Finf?= =?UTF-8?q?o).=20-=20Models=20role,=20task-file,=20task,=20handler,=20vari?= =?UTF-8?q?able,=20and=20external-role=20nodes.=20-=20Models=20typed=20inc?= =?UTF-8?q?lude/import,=20role-call,=20notify,=20variable-use,=20and=20con?= =?UTF-8?q?tainment=20edges.=20-=20Preserves=20static,=20dynamic,=20unknow?= =?UTF-8?q?n,=20and=20unresolved=5Fexternal=20resolution.=20-=20Uses=20raw?= =?UTF-8?q?=20loaded=20Ansible=20tasks,=20so=20no=20second=20parser=20or?= =?UTF-8?q?=20NetworkX=20dependency.=20README=20projections=20-=20Executio?= =?UTF-8?q?n=20phases=20now=20traverse=20static=20relationships=20from=20t?= =?UTF-8?q?asks/main.yml.=20-=20Nginx=20now=20starts=20at=20main.yml,=20sh?= =?UTF-8?q?ows=20seven=20OS=20branches=20with=20their=20actual=20condition?= =?UTF-8?q?s,=20then=20vhosts.yml.=20-=20Variable=20and=20handler=20diagra?= =?UTF-8?q?m=20edges=20are=20source-backed.=20-=20Removed=20the=20remainin?= =?UTF-8?q?g=20first-file=20variable=20and=20last-file=20handler=20proxy?= =?UTF-8?q?=20behavior=20during=20normal=20rendering.=201.=20Loop=20metada?= =?UTF-8?q?ta=20-=20Ordinary=20task=20nodes=20now=20retain=20loop,=20with?= =?UTF-8?q?=5Fitems,=20with=5Ffirst=5Ffound,=20etc.=20-=20README=20task=20?= =?UTF-8?q?tables=20show=20a=20conditional=20Loop=20column.=20-=20MySQL=20?= =?UTF-8?q?now=20displays=20all=20legacy=20loops.=20-=20Commit=20message:?= =?UTF-8?q?=20feat:=20render=20task=20loop=20metadata=202.=20Graph-derived?= =?UTF-8?q?=20complexity=20-=20Structural=20metrics=20remain=20unchanged.?= =?UTF-8?q?=20-=20Added=20graph=20metrics:=20static=20reachable=20files,?= =?UTF-8?q?=20dynamic/unknown=20boundaries,=20external=20roles,=20loop=20t?= =?UTF-8?q?asks,=20notification=20edges,=20and=20orphan=20files.=20-=20MyS?= =?UTF-8?q?QL=20reports=2010=20reachable=20files,=2011=20loop=20tasks,=202?= =?UTF-8?q?=20notification=20edges,=200=20orphans.=20-=20Commit=20message:?= =?UTF-8?q?=20feat:=20add=20execution=20graph=20complexity=20metrics=203.?= =?UTF-8?q?=20Execution=20routes=20-=20Replaced=20placeholder=20=E2=80=9CP?= =?UTF-8?q?hase=E2=80=9D=20prose=20with=20source-backed=20Execution=20Rout?= =?UTF-8?q?es.=20-=20Shows=20entry=20point,=20static=20continuation,=20con?= =?UTF-8?q?ditional=20paths,=20and=20unreachable=20files=20without=20prete?= =?UTF-8?q?nding=20branches=20all=20execute.=20-=20Commit=20message:=20fea?= =?UTF-8?q?t:=20replace=20phase=20placeholders=20with=20execution=20routes?= =?UTF-8?q?=204.=20Role=20boundaries=20and=20renderer=20contract=20-=20Sta?= =?UTF-8?q?tic=20import=5Frole=20targets=20become=20explicit=20external-ro?= =?UTF-8?q?le=20reference=20nodes.=20-=20Dynamic=20include=5Frole=20target?= =?UTF-8?q?s=20remain=20unresolved/dynamic.=20-=20RoleExecutionGraph.to=5F?= =?UTF-8?q?dict()=20is=20JSON=20serializable=20and=20tested=20as=20the=20r?= =?UTF-8?q?enderer=20contract.=20-=20Commit=20message:=20feat:=20expose=20?= =?UTF-8?q?role=20execution=20graph=20boundaries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAIMS.md | 56 +++++++++++++++++-- .../analyzers/role_analyzer.py | 26 +++++++++ .../analyzers/complexity_analyzer/models.py | 9 +++ docsible/graphs/role_execution.py | 5 +- .../role/sections/adaptive_diagrams.jinja2 | 26 ++++----- docsible/templates/role/sections/tasks.jinja2 | 13 +++-- .../role/sections/tasks_hybrid.jinja2 | 9 +-- docsible/utils/special_tasks_keys.py | 5 ++ .../test_adaptive_documentation.py | 6 +- tests/graphs/test_role_execution.py | 33 +++++++++++ tests/utils/test_special_tasks_keys.py | 7 +++ 11 files changed, 162 insertions(+), 33 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index db15a93..2634f59 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -1,6 +1,6 @@ # Docsible: Verified Project State -Snapshot: 2026-08-27 +Snapshot: 2026-09-13 ## Purpose @@ -77,6 +77,53 @@ npx --yes jscpd docsible - The deprecated `docsible role` command is still present alongside the newer intent-based command groups. +## Role Execution Graph + +Docsible now has an internal, renderer-independent `RoleExecutionGraph` built +from the raw Ansible task facts already retained by `RoleInfoLoader`. Its small +interface is `build_role_execution_graph(role_info)`. + +- Nodes represent roles, task files, tasks, handlers, variables, and external + role references. +- Typed edges represent containment, task-file include/import, role + include/import, task-to-handler notification, and known-variable use. +- Every relationship carries its source location and preserves the resolution + state: `static`, `dynamic`, `unknown`, or `unresolved_external`. Dynamic + Ansible expressions are recorded without inventing a target. +- README execution phases are now a static traversal from `tasks/main.yml`; + conditional paths are annotated and unreachable files are identified rather + than being presented as filesystem-order phases. +- Component architecture diagrams derive variable and handler edges from graph + facts, replacing the old first-file and last-file proxy edges. +- The graph uses standard-library dataclasses for a small serializable core. + NetworkX is not a Docsible dependency; a future visualization adapter may + convert the graph for layout algorithms. + +### Verified External Cases + +- `geerlingguy/ansible-role-docker` @ `38be616950679548ae0ba8a81ffcceca1b3090bd`: + JSON parses, documentation generation succeeds, `main.yml` is Phase 1, and + the five conditional include boundaries plus actual handler notifications + render as source-backed relationships. +- `geerlingguy/ansible-role-nginx` @ `5ff0b235006390a0d5666fd4cce7477410982cdf`: + JSON parses, documentation generation succeeds, `main.yml` is Phase 1, the + seven OS-specific branches retain their `when` conditions, and `vhosts.yml` + is reached through its static import. + +### Next Graph Milestones + +1. Publish a documented JSON graph contract after its node and edge fields are + exercised by more external candidates. +2. Resolve locally available roles in sibling role directories and collections; + retain absent Galaxy/FQCN roles as explicit external-reference nodes. +3. Add graph projections for dynamic task/role includes, loops, blocks, + rescue/always, and source-linked variable scopes without claiming static + certainty where Ansible defers resolution. +4. Make `graph_visualisation` a renderer adapter over this contract, using + NetworkX only for renderer-specific layout work. +5. Extend the pinned external corpus before treating the graph contract as + release-stable. + ## Remaining Duplication Work The source-only duplication scan is below the original baseline, but remaining @@ -94,6 +141,7 @@ duplication is prioritized by ownership and behavior rather than percentage. ## Scope of This Document -This file records observable project state and commands verified for this -snapshot. It does not assert historical phase completion, performance results, -future roadmaps, or unverified feature maturity. +This file records observable project state, commands verified for this +snapshot, and explicitly approved next milestones for the Role Execution Graph. +It does not assert historical phase completion, performance results, or +unverified feature maturity. diff --git a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py index 3262d23..254dd73 100644 --- a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py +++ b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py @@ -203,6 +203,31 @@ def analyze_role_complexity( inflection_points = detect_inflection_points(role_info, hotspots) # Create metrics + from docsible.graphs import EdgeKind, NodeKind, ResolutionStatus, build_role_execution_graph + + execution_graph = build_role_execution_graph(role_info) + phases = execution_graph.execution_phases() + graph_metrics = { + "static_reachable_task_files": sum(phase["kind"] != "unreachable" for phase in phases), + "dynamic_boundaries": sum( + edge.resolution is ResolutionStatus.DYNAMIC for edge in execution_graph.edges + ), + "unknown_boundaries": sum( + edge.resolution is ResolutionStatus.UNKNOWN for edge in execution_graph.edges + ), + "external_role_references": sum( + node.kind is NodeKind.EXTERNAL_ROLE for node in execution_graph.nodes.values() + ), + "loop_tasks": sum( + node.kind is NodeKind.TASK and "loop" in node.metadata + for node in execution_graph.nodes.values() + ), + "notification_edges": sum( + edge.kind is EdgeKind.NOTIFIES_HANDLER and edge.target_id is not None + for edge in execution_graph.edges + ), + "orphan_task_files": sum(phase["kind"] == "unreachable" for phase in phases), + } metrics = ComplexityMetrics( total_tasks=total_tasks, task_files=task_files, @@ -215,6 +240,7 @@ def analyze_role_complexity( external_integrations=len(integration_points), max_tasks_per_file=max_tasks_per_file, avg_tasks_per_file=avg_tasks_per_file, + **graph_metrics, ) # Classify complexity diff --git a/docsible/analyzers/complexity_analyzer/models.py b/docsible/analyzers/complexity_analyzer/models.py index ad0963d..f6270ab 100644 --- a/docsible/analyzers/complexity_analyzer/models.py +++ b/docsible/analyzers/complexity_analyzer/models.py @@ -63,6 +63,15 @@ class ComplexityMetrics(BaseModel): role_includes: int = Field(default=0, description="include_role/import_role count") task_includes: int = Field(default=0, description="include_tasks/import_tasks count") + # Execution graph metrics (source-backed relationships, not runtime claims) + static_reachable_task_files: int = Field(default=0) + dynamic_boundaries: int = Field(default=0) + unknown_boundaries: int = Field(default=0) + external_role_references: int = Field(default=0) + loop_tasks: int = Field(default=0) + notification_edges: int = Field(default=0) + orphan_task_files: int = Field(default=0) + # External integrations external_integrations: int = Field( default=0, description="Count of external system connections" diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index e32183b..19ae5f3 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -219,7 +219,10 @@ def _add_tasks(graph: RoleExecutionGraph, role_name: str, task_file: dict[str, A line = line_ranges[index[0]][0] if index and index[0] < len(line_ranges) else None source = SourceLocation(f"tasks/{file_name}", line) task_id = f"task:{role_name}:{file_name}:{'.'.join(map(str, index))}" - graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, {"file": file_name, "module": _module_name(task)})) + metadata = {"file": file_name, "module": _module_name(task)} + if loop := _loop(task): + metadata["loop"] = loop + graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, metadata)) graph.add_edge(GraphEdge(EdgeKind.CONTAINS, file_ids[file_name], task_id, ResolutionStatus.STATIC, source)) _add_variable_edges(graph, task_id, task, variables, source) _add_notification_edges(graph, task_id, task, handlers, source) diff --git a/docsible/templates/role/sections/adaptive_diagrams.jinja2 b/docsible/templates/role/sections/adaptive_diagrams.jinja2 index 9a50d0b..61be31e 100644 --- a/docsible/templates/role/sections/adaptive_diagrams.jinja2 +++ b/docsible/templates/role/sections/adaptive_diagrams.jinja2 @@ -88,28 +88,22 @@ This role integrates with {{ complexity_report.integration_points|length }} exte {% endfor %} {% endif %} -### Execution Phases +### Execution Routes -These phases are a static traversal from the role entry point. Conditional -branches are annotated; dynamic includes remain unresolved. +Static traversal from the role entry point. Conditional paths do not imply that +all branches run in one execution; dynamic boundaries remain unresolved. {% for task_file_info in execution_phases or complexity_report.task_files_detail %} -#### Phase {{ loop.index }}: {{ task_file_info.file }} -**Tasks:** {{ task_file_info.task_count }} +{% if loop.first %}#### Entry point: {{ task_file_info.file }} +{% elif task_file_info.kind is defined and task_file_info.kind == 'conditional' %}#### Conditional path: {{ task_file_info.file }} +{% elif task_file_info.kind is defined and task_file_info.kind == 'unreachable' %}#### Unreached static file: {{ task_file_info.file }} +{% else %}#### Static continuation: {{ task_file_info.file }} +{% endif %} +Tasks: {{ task_file_info.task_count }} {% if task_file_info.kind is defined and task_file_info.kind == 'conditional' %} -**Conditional path:** {{ task_file_info.conditions | join(' and ') }} -{% elif task_file_info.kind is defined and task_file_info.kind == 'unreachable' %} -**Static traversal:** not reached from the entry point +Condition: `{{ task_file_info.conditions | join(' and ') }}` {% endif %} -> **Note:** Document decision points, error handling, and key operations for this phase -> -> Example structure: -> - **Purpose:** What this phase accomplishes -> - **Prerequisites:** What must be true before this phase -> - **Decision Points:** Key conditional logic -> - **Error Handling:** How failures are managed - {% endfor %} ### Recommendations diff --git a/docsible/templates/role/sections/tasks.jinja2 b/docsible/templates/role/sections/tasks.jinja2 index 3a2952b..8fd828b 100644 --- a/docsible/templates/role/sections/tasks.jinja2 +++ b/docsible/templates/role/sections/tasks.jinja2 @@ -1,19 +1,22 @@ {% for task_file in role.tasks %} #### File: tasks/{{ task_file.file }} -{%- set ns = namespace(has_tags=false, has_comments=false) -%} +{%- set ns = namespace(has_tags=false, has_comments=false, has_loops=false) -%} {%- for task in task_file.tasks -%} {%- if task_file.mermaid | selectattr('name', 'equalto', task.name) | map(attribute='tags') | list | first -%} {%- set ns.has_tags = true -%} {%- endif -%} {%- endfor -%} +{%- if task_file.tasks | selectattr('loop', 'defined') | list -%} + {%- set ns.has_loops = true -%} +{%- endif -%} {%- if task_file.comments | length > 0 -%} {%- set ns.has_comments = true -%} {%- endif %} -| Task Name | Module | Has Conditions |{% if ns.has_tags %} Tags |{% endif %}{% if ns.has_comments %} Description |{% endif %} -|-----------|--------|----------------|{% if ns.has_tags %}------|{% endif %}{% if ns.has_comments %}-------------|{% endif %} +| Task Name | Module | Has Conditions |{% if ns.has_loops %} Loop |{% endif %}{% if ns.has_tags %} Tags |{% endif %}{% if ns.has_comments %} Description |{% endif %} +|-----------|--------|----------------|{% if ns.has_loops %}------|{% endif %}{% if ns.has_tags %}------|{% endif %}{% if ns.has_comments %}-------------|{% endif %} {% for task in task_file.tasks -%} {%- set link = links.render_repo_link(role.repository, role.name, 'tasks/' ~ task_file.file, task_file.lines[task.name], role.repository_type, role.repository_branch, role.belongs_to_collection) -%} -| {% if task_file.lines and task_file.lines[task.name] %}[{{ task.name | escape_table_cell }}]({{ link }}){% else %}{{ task.name | escape_table_cell }}{% endif %} | {{ task.module | escape_table_cell }} | {{ 'True' if task.when else 'False' }} |{% if ns.has_tags %} {{ task_file.mermaid | selectattr('name', 'equalto', task.name) | map(attribute='tags') | safe_join(',') }} |{% endif %}{% if ns.has_comments %} {{ task_file.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} +| {% if task_file.lines and task_file.lines[task.name] %}[{{ task.name | escape_table_cell }}]({{ link }}){% else %}{{ task.name | escape_table_cell }}{% endif %} | {{ task.module | escape_table_cell }} | {{ 'True' if task.when else 'False' }} |{% if ns.has_loops %} {{ task.loop | default('') | escape_table_cell }} |{% endif %}{% if ns.has_tags %} {{ task_file.mermaid | selectattr('name', 'equalto', task.name) | map(attribute='tags') | safe_join(',') }} |{% endif %}{% if ns.has_comments %} {{ task_file.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} {% endfor %} {# Mermaid diagram section with pagination support #} @@ -52,4 +55,4 @@ {% endif %} {% endif %} -{% endfor %} \ No newline at end of file +{% endfor %} diff --git a/docsible/templates/role/sections/tasks_hybrid.jinja2 b/docsible/templates/role/sections/tasks_hybrid.jinja2 index 78e4c4d..2af80c2 100644 --- a/docsible/templates/role/sections/tasks_hybrid.jinja2 +++ b/docsible/templates/role/sections/tasks_hybrid.jinja2 @@ -1,12 +1,13 @@ {% for task_info in role.tasks %} ### File: `tasks/{{ task_info.file }}` {% if task_info.tasks %} -{% set ns = namespace(has_comments=false) %} +{% set ns = namespace(has_comments=false, has_loops=false) %} {% if task_info.comments | length > 0 %}{% set ns.has_comments = true %}{% endif %} -| Task Name | Module |{% if ns.has_comments %} Description |{% endif %} -|-----------|--------|{% if ns.has_comments %}-------------|{% endif %} +{% if task_info.tasks | selectattr('loop', 'defined') | list %}{% set ns.has_loops = true %}{% endif %} +| Task Name | Module |{% if ns.has_loops %} Loop |{% endif %}{% if ns.has_comments %} Description |{% endif %} +|-----------|--------|{% if ns.has_loops %}------|{% endif %}{% if ns.has_comments %}-------------|{% endif %} {%- for task in task_info.tasks %} -| {{ task.name | escape_table_cell if task.name else '*unnamed*' }} | {{ task.module | escape_table_cell }} |{% if ns.has_comments %} {{ task_info.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} +| {{ task.name | escape_table_cell if task.name else '*unnamed*' }} | {{ task.module | escape_table_cell }} |{% if ns.has_loops %} {{ task.loop | default('') | escape_table_cell }} |{% endif %}{% if ns.has_comments %} {{ task_info.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} {%- endfor %} {% endif %} diff --git a/docsible/utils/special_tasks_keys.py b/docsible/utils/special_tasks_keys.py index 2132cef..513b135 100644 --- a/docsible/utils/special_tasks_keys.py +++ b/docsible/utils/special_tasks_keys.py @@ -209,6 +209,11 @@ def process_special_task_keys( "type": task_type, "when": task_when, } + loop_key = "loop" if "loop" in task else next( + (key for key in task if key.startswith("with_")), None + ) + if loop_key is not None: + processed_task["loop"] = loop_key if include_target is not None: processed_task["include_target"] = include_target tasks.append(processed_task) diff --git a/tests/documentation/test_adaptive_documentation.py b/tests/documentation/test_adaptive_documentation.py index a4aa435..df9fc5f 100644 --- a/tests/documentation/test_adaptive_documentation.py +++ b/tests/documentation/test_adaptive_documentation.py @@ -116,9 +116,9 @@ def test_complex_role_shows_architecture_overview(self): assert "## Architecture Overview" in result assert "### Role Components" in result assert "30 tasks" in result - assert "### Execution Phases" in result - assert "Phase 1: install.yml" in result - assert "Phase 2: configure.yml" in result + assert "### Execution Routes" in result + assert "Entry point: install.yml" in result + assert "Static continuation: configure.yml" in result # Should show recommendations assert "### Recommendations" in result assert "Role is complex" in result diff --git a/tests/graphs/test_role_execution.py b/tests/graphs/test_role_execution.py index 73c1219..c766884 100644 --- a/tests/graphs/test_role_execution.py +++ b/tests/graphs/test_role_execution.py @@ -1,5 +1,7 @@ """Tests for the source-backed role execution graph.""" +import json + from docsible.graphs import EdgeKind, ResolutionStatus, build_role_execution_graph @@ -46,3 +48,34 @@ def test_builds_static_dynamic_and_notify_relationships(): phases = graph.execution_phases() assert [phase["file"] for phase in phases] == ["main.yml", "setup.yml"] assert phases[1]["kind"] == "static" + + +def test_preserves_external_role_boundaries_in_renderer_contract(): + graph = build_role_execution_graph( + { + "name": "web", + "defaults": [], + "vars": [], + "handlers": [], + "tasks": [ + { + "file": "main.yml", + "tasks": [{}, {}], + "line_ranges": [(1, 2), (3, 4)], + "mermaid": [ + {"import_role": {"name": "vendor.common", "tasks_from": "setup"}}, + {"include_role": "{{ selected_role }}"}, + ], + } + ], + } + ) + + role_edges = [ + edge for edge in graph.edges if edge.kind in {EdgeKind.IMPORTS_ROLE, EdgeKind.INCLUDES_ROLE} + ] + assert role_edges[0].resolution is ResolutionStatus.UNRESOLVED_EXTERNAL + assert role_edges[0].target_id == "external_role:vendor.common" + assert role_edges[1].resolution is ResolutionStatus.DYNAMIC + assert role_edges[1].target_id is None + json.dumps(graph.to_dict()) diff --git a/tests/utils/test_special_tasks_keys.py b/tests/utils/test_special_tasks_keys.py index f4380d7..d20d597 100644 --- a/tests/utils/test_special_tasks_keys.py +++ b/tests/utils/test_special_tasks_keys.py @@ -75,6 +75,13 @@ def test_include_role_target_captured(): assert result[0]["include_target"] == "common" +def test_loop_syntax_is_preserved_on_regular_tasks(): + modern = process_special_task_keys({"debug": {"msg": "{{ item }}"}, "loop": ["a"]}) + legacy = process_special_task_keys({"debug": {"msg": "{{ item }}"}, "with_items": ["a"]}) + assert modern[0]["loop"] == "loop" + assert legacy[0]["loop"] == "with_items" + + def test_block_task_shape_unchanged(): result = process_special_task_keys( {"name": "Handle failure", "block": [{"debug": {"msg": "try"}}], "rescue": []} From 79cf7e9acfd646e164311a2bb1b96927a34db303 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 02:01:35 +0200 Subject: [PATCH 05/20] updated claims to show role execution graph work in progress --- CLAIMS.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 2634f59..e0aaa3e 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -92,9 +92,16 @@ interface is `build_role_execution_graph(role_info)`. Ansible expressions are recorded without inventing a target. - README execution phases are now a static traversal from `tasks/main.yml`; conditional paths are annotated and unreachable files are identified rather - than being presented as filesystem-order phases. + than being presented as filesystem-order phases. The README presents these + as Execution Routes rather than placeholder phases. - Component architecture diagrams derive variable and handler edges from graph facts, replacing the old first-file and last-file proxy edges. +- Task nodes preserve modern and legacy loop syntax (`loop`, `with_items`, + `with_first_found`, and other `with_*` forms); README task tables render a + Loop column when applicable. +- Complexity reports retain their structural metrics and add graph metrics: + statically reachable task files, dynamic and unknown boundaries, external + role references, loop tasks, notification edges, and orphan task files. - The graph uses standard-library dataclasses for a small serializable core. NetworkX is not a Docsible dependency; a future visualization adapter may convert the graph for layout algorithms. @@ -110,13 +117,21 @@ interface is `build_role_execution_graph(role_info)`. seven OS-specific branches retain their `when` conditions, and `vhosts.yml` is reached through its static import. +### Completed Graph Milestones + +1. Preserve loop metadata on ordinary tasks and render it in documentation. +2. Add graph-derived execution metrics alongside structural complexity counts. +3. Replace placeholder phases with source-backed Execution Routes. +4. Preserve static and dynamic cross-role boundaries in a JSON-serializable + renderer contract. + ### Next Graph Milestones 1. Publish a documented JSON graph contract after its node and edge fields are exercised by more external candidates. 2. Resolve locally available roles in sibling role directories and collections; retain absent Galaxy/FQCN roles as explicit external-reference nodes. -3. Add graph projections for dynamic task/role includes, loops, blocks, +3. Add graph projections for dynamic task/role includes, blocks, rescue/always, and source-linked variable scopes without claiming static certainty where Ansible defers resolution. 4. Make `graph_visualisation` a renderer adapter over this contract, using From aefa37cac0449e73a552443cfda4282ee5cc020e Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 02:02:05 +0200 Subject: [PATCH 06/20] added fix branch to ci step --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7725568..97ff4f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: CI on: push: - branches: [main, "feature/**"] + branches: [main, "feature/**", "fix/**"] pull_request: branches: [main] From 6c440aa017a9f247562d049a2bc30255e0328974 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 02:47:09 +0200 Subject: [PATCH 07/20] - Safe nested target resolution: - Resolves only {{ role_path }}/tasks/ into a static task-file boundary. - Leaves every other templated target dynamic. - Prevents safe nested includes from becoming false orphans. - Enterprise projection policy: - Detailed task-file architecture budget: - max 20 files - max 35 boundaries - max fan-out 8 - Exceeding any signal selects a grouped directory-level execution overview. - The full graph remains intact; only the README projection changes. - Dynamic/unknown source groups and handler relationships remain visible. - Enterprise routes collapse individual orphan files into one summary count. - Enterprise graph availability: - ENTERPRISE roles now receive graph summaries through smart defaults. - Explicitly disabling smart defaults still suppresses graphs, preserving the existing contract. --- docsible/defaults/decisions/graph_rule.py | 5 +- docsible/diagrams/types/architecture.py | 93 ++++++++++++++++++- docsible/graphs/role_execution.py | 6 +- .../role/sections/adaptive_diagrams.jinja2 | 16 +++- 4 files changed, 114 insertions(+), 6 deletions(-) diff --git a/docsible/defaults/decisions/graph_rule.py b/docsible/defaults/decisions/graph_rule.py index 5b6bdd0..87aa24b 100644 --- a/docsible/defaults/decisions/graph_rule.py +++ b/docsible/defaults/decisions/graph_rule.py @@ -64,7 +64,10 @@ def decide(self, context: DecisionContext) -> Decision | None: ) # 3. Complex role → highly recommend graphs - if context.is_complex_role or context.task_file_count > MAX_TASK_FILES_LOWER_BOUND: + if ( + context.complexity_category in {"complex", "enterprise"} + or context.task_file_count > MAX_TASK_FILES_LOWER_BOUND + ): return Decision( option_name="generate_graph", value=True, diff --git a/docsible/diagrams/types/architecture.py b/docsible/diagrams/types/architecture.py index f6e6af3..d03e1ec 100644 --- a/docsible/diagrams/types/architecture.py +++ b/docsible/diagrams/types/architecture.py @@ -5,10 +5,15 @@ """ import logging +import re from typing import Any logger = logging.getLogger(__name__) +_DETAILED_FILE_BUDGET = 20 +_DETAILED_EDGE_BUDGET = 35 +_DETAILED_FANOUT_BUDGET = 8 + def generate_component_architecture( role_info: dict[str, Any] | None, complexity_report: Any, execution_graph: Any | None = None @@ -72,6 +77,8 @@ def generate_component_architecture( # Tasks subgraph with file breakdown task_files = role_info.get("tasks", []) + if execution_graph is not None and _should_group_execution_graph(execution_graph, task_files): + return _generate_grouped_architecture(role_info, execution_graph) if task_files: lines.append(" subgraph Tasks") for idx, task_file in enumerate(task_files): @@ -219,6 +226,90 @@ def generate_component_architecture( return "\n".join(lines) +def _should_group_execution_graph(execution_graph: Any, task_files: list[dict[str, Any]]) -> bool: + include_edges = [ + edge + for edge in execution_graph.edges + if edge.kind.value in {"includes_task_file", "imports_task_file"} + ] + fanout: dict[str, int] = {} + for edge in include_edges: + fanout[edge.source_id] = fanout.get(edge.source_id, 0) + 1 + return ( + len(task_files) > _DETAILED_FILE_BUDGET + or len(include_edges) > _DETAILED_EDGE_BUDGET + or max(fanout.values(), default=0) > _DETAILED_FANOUT_BUDGET + ) + + +def _generate_grouped_architecture(role_info: dict[str, Any], execution_graph: Any) -> str: + """Render a bounded directory-level overview from graph facts.""" + task_nodes = [node for node in execution_graph.nodes.values() if node.kind.value == "task_file"] + groups: dict[str, list[str]] = {} + for node in task_nodes: + file_name = node.metadata["file"] + group = "entry point" if file_name == "main.yml" else file_name.split("/", 1)[0] + groups.setdefault(group, []).append(file_name) + + def node_id(group: str) -> str: + return "group_" + re.sub(r"[^A-Za-z0-9_]", "_", group) + + file_group = {file_name: group for group, files in groups.items() for file_name in files} + lines = ["graph TB", ' overview["Grouped execution overview"]'] + for group, files in sorted(groups.items()): + label = "main.yml" if group == "entry point" else f"{group}
{len(files)} task files" + lines.append(f' {node_id(group)}["{label}"]') + lines.append(f" overview --> {node_id(group)}") + + task_to_file = { + edge.target_id: edge.source_id + for edge in execution_graph.edges + if edge.kind.value == "contains" and edge.source_id.startswith("task_file:") + } + file_by_id = {node.id: node.metadata["file"] for node in task_nodes} + grouped_edges: dict[tuple[str, str], int] = {} + uncertain_sources: set[str] = set() + notify_sources: set[str] = set() + for edge in execution_graph.edges: + if edge.kind.value in {"includes_task_file", "imports_task_file"}: + source_file_id = task_to_file.get(edge.source_id) + source_file = file_by_id.get(source_file_id) + target_file = file_by_id.get(edge.target_id) + if source_file and target_file: + key = (file_group[source_file], file_group[target_file]) + grouped_edges[key] = grouped_edges.get(key, 0) + 1 + elif source_file and edge.resolution.value in {"dynamic", "unknown"}: + uncertain_sources.add(file_group[source_file]) + elif edge.kind.value == "notifies_handler" and edge.target_id: + source_file_id = task_to_file.get(edge.source_id) + if source_file_id in file_by_id: + notify_sources.add(file_group[file_by_id[source_file_id]]) + + for (source, target), count in sorted(grouped_edges.items()): + if source != target: + label = "include" if count == 1 else f"{count} includes" + lines.append(f' {node_id(source)} -."{label}".-> {node_id(target)}') + if uncertain_sources: + lines.append(f' uncertain["Dynamic or unknown
{len(uncertain_sources)} source groups"]') + for source in sorted(uncertain_sources): + lines.append(f' {node_id(source)} -."dynamic".-> uncertain') + if notify_sources: + lines.append(' handlers["Handlers"]') + for source in sorted(notify_sources): + lines.append(f' {node_id(source)} -."notify".-> handlers') + lines.extend( + [ + " classDef groupStyle fill:#f3e5f5,stroke:#7b1fa2,stroke-width:2px", + " classDef uncertaintyStyle fill:#fff3e0,stroke:#f57c00,stroke-width:2px", + " class overview groupStyle", + " class uncertain uncertaintyStyle" if uncertain_sources else "", + ] + ) + for group in groups: + lines.append(f" class {node_id(group)} groupStyle") + return "\n".join(line for line in lines if line) + + def should_generate_architecture_diagram(complexity_report: Any) -> bool: """ Determine if a component architecture diagram should be generated. @@ -239,7 +330,7 @@ def should_generate_architecture_diagram(complexity_report: Any) -> bool: return False # Generate for COMPLEX roles - if complexity_report.category.value == "complex": + if complexity_report.category.value in {"complex", "enterprise"}: return True # Generate for MEDIUM roles with high composition diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index 19ae5f3..1f46c8d 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -153,6 +153,7 @@ def to_dict(self) -> dict[str, Any]: _TASK_FILE_ACTIONS = {"include", "include_tasks", "import_tasks"} _ROLE_ACTIONS = {"include_role", "import_role"} _VARIABLE_PATTERN = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\b") +_ROLE_PATH_TASK_PREFIX = re.compile(r"^\{\{\s*role_path\s*\}\}/tasks/") def build_role_execution_graph(role_info: dict[str, Any]) -> RoleExecutionGraph: @@ -271,7 +272,8 @@ def _add_composition_edge(graph: RoleExecutionGraph, task_id: str, task: dict[st if isinstance(raw_target, dict): raw_target = raw_target.get("file" if short_module in _TASK_FILE_ACTIONS else "name") target = str(raw_target) if raw_target is not None else "" - dynamic = "{{" in target or "{%" in target + resolved_target = _ROLE_PATH_TASK_PREFIX.sub("", target) + dynamic = "{{" in resolved_target or "{%" in resolved_target kind = EdgeKind.IMPORTS_TASK_FILE if short_module == "import_tasks" else EdgeKind.INCLUDES_TASK_FILE if short_module in _ROLE_ACTIONS: kind = EdgeKind.IMPORTS_ROLE if short_module == "import_role" else EdgeKind.INCLUDES_ROLE @@ -280,7 +282,7 @@ def _add_composition_edge(graph: RoleExecutionGraph, task_id: str, task: dict[st graph.add_node(GraphNode(target_id, NodeKind.EXTERNAL_ROLE, target, metadata={"role": target})) resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.UNRESOLVED_EXTERNAL else: - target_id = _resolve_task_file(target, source_file, file_ids) if not dynamic else None + target_id = _resolve_task_file(resolved_target, source_file, file_ids) if not dynamic else None resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.STATIC if target_id else ResolutionStatus.UNKNOWN graph.add_edge(GraphEdge(kind, task_id, target_id, resolution, source, condition=_condition(task), loop=_loop(task), target_expression=target or None)) diff --git a/docsible/templates/role/sections/adaptive_diagrams.jinja2 b/docsible/templates/role/sections/adaptive_diagrams.jinja2 index 61be31e..eb4f01b 100644 --- a/docsible/templates/role/sections/adaptive_diagrams.jinja2 +++ b/docsible/templates/role/sections/adaptive_diagrams.jinja2 @@ -47,7 +47,7 @@ This state diagram shows the role's execution phases and decision points: {% endif %} {# COMPLEX roles (25+ tasks): Use architecture + text documentation #} -{% if complexity_report.category.value == 'complex' %} +{% if complexity_report.category.value in ['complex', 'enterprise'] %} ## Architecture Overview @@ -93,7 +93,11 @@ This role integrates with {{ complexity_report.integration_points|length }} exte Static traversal from the role entry point. Conditional paths do not imply that all branches run in one execution; dynamic boundaries remain unresolved. -{% for task_file_info in execution_phases or complexity_report.task_files_detail %} +{% set route_files = execution_phases or complexity_report.task_files_detail %} +{% if complexity_report.category.value == 'enterprise' %} +{% set route_files = route_files | rejectattr('kind', 'equalto', 'unreachable') | list %} +{% endif %} +{% for task_file_info in route_files %} {% if loop.first %}#### Entry point: {{ task_file_info.file }} {% elif task_file_info.kind is defined and task_file_info.kind == 'conditional' %}#### Conditional path: {{ task_file_info.file }} {% elif task_file_info.kind is defined and task_file_info.kind == 'unreachable' %}#### Unreached static file: {{ task_file_info.file }} @@ -105,6 +109,14 @@ Condition: `{{ task_file_info.conditions | join(' and ') }}` {% endif %} {% endfor %} +{% if complexity_report.category.value == 'enterprise' and execution_phases %} +{% set orphan_count = execution_phases | selectattr('kind', 'equalto', 'unreachable') | list | length %} +{% if orphan_count %} +#### Unreached static files: {{ orphan_count }} +These files are not reached through statically resolvable boundaries from the entry point. + +{% endif %} +{% endif %} ### Recommendations {% for recommendation in complexity_report.recommendations %} From 2f999ae24e4f46d98f60044e1c35a72fbd80b0e0 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 03:08:31 +0200 Subject: [PATCH 08/20] What changed - Resolves safe {{ role_path }}/tasks/ paths as static. - Resolves templated paths with a literal filename prefix, such as install-{{ os_family }}.yml, into multiple dynamic candidate edges. - Keeps unconstrained expressions like {{ unknown }}.yml dynamic without inventing candidates. - Dynamic candidate files are no longer false orphans. - ENTERPRISE output uses the grouped execution overview under the agreed readability budget. - Dynamic candidates are summarized, not listed one-by-one. - Explicitly disabling smart defaults remains graph-free. Official nginx recalibration - Dynamic boundaries: 22 - Unknown boundaries: 0 - Statically reachable task files: 15 - False orphans: 0 --- .../analyzers/role_analyzer.py | 12 +++- docsible/diagrams/types/architecture.py | 2 +- docsible/graphs/role_execution.py | 57 +++++++++++++++++-- .../role/sections/adaptive_diagrams.jinja2 | 11 ++++ 4 files changed, 74 insertions(+), 8 deletions(-) diff --git a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py index 254dd73..38a2e4b 100644 --- a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py +++ b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py @@ -208,12 +208,18 @@ def analyze_role_complexity( execution_graph = build_role_execution_graph(role_info) phases = execution_graph.execution_phases() graph_metrics = { - "static_reachable_task_files": sum(phase["kind"] != "unreachable" for phase in phases), + "static_reachable_task_files": sum( + phase["kind"] in {"entrypoint", "static", "conditional"} for phase in phases + ), "dynamic_boundaries": sum( - edge.resolution is ResolutionStatus.DYNAMIC for edge in execution_graph.edges + edge.resolution is ResolutionStatus.DYNAMIC + and edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE, EdgeKind.INCLUDES_ROLE, EdgeKind.IMPORTS_ROLE} + for edge in execution_graph.edges ), "unknown_boundaries": sum( - edge.resolution is ResolutionStatus.UNKNOWN for edge in execution_graph.edges + edge.resolution is ResolutionStatus.UNKNOWN + and edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE, EdgeKind.INCLUDES_ROLE, EdgeKind.IMPORTS_ROLE} + for edge in execution_graph.edges ), "external_role_references": sum( node.kind is NodeKind.EXTERNAL_ROLE for node in execution_graph.nodes.values() diff --git a/docsible/diagrams/types/architecture.py b/docsible/diagrams/types/architecture.py index d03e1ec..1e6de70 100644 --- a/docsible/diagrams/types/architecture.py +++ b/docsible/diagrams/types/architecture.py @@ -278,7 +278,7 @@ def node_id(group: str) -> str: if source_file and target_file: key = (file_group[source_file], file_group[target_file]) grouped_edges[key] = grouped_edges.get(key, 0) + 1 - elif source_file and edge.resolution.value in {"dynamic", "unknown"}: + if source_file and edge.resolution.value in {"dynamic", "unknown"}: uncertain_sources.add(file_group[source_file]) elif edge.kind.value == "notifies_handler" and edge.target_id: source_file_id = task_to_file.get(edge.source_id) diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index 1f46c8d..3fac37b 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -6,6 +6,7 @@ from collections import deque from dataclasses import asdict, dataclass, field from enum import Enum +from fnmatch import fnmatch from pathlib import PurePosixPath from typing import Any @@ -98,7 +99,7 @@ def execution_phases(self) -> list[dict[str, Any]]: for edge in self.edges: if ( edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE} - and edge.resolution is ResolutionStatus.STATIC + and edge.resolution in {ResolutionStatus.STATIC, ResolutionStatus.DYNAMIC} and edge.target_id in file_ids ): source_file_id = task_files_by_task.get(edge.source_id) @@ -122,12 +123,23 @@ def execution_phases(self) -> list[dict[str, Any]]: "task_count": node.metadata["task_count"], "kind": phase_kind, "conditions": sorted({edge.condition for edge in incoming if edge.condition}), + "expressions": sorted( + {edge.target_expression for edge in incoming if edge.target_expression} + ), } ) for edge in outgoing.get(node_id, []): if edge.target_id is not None: queue.append( - (edge.target_id, "conditional" if edge.condition else "static", [edge]) + ( + edge.target_id, + "dynamic" + if phase_kind == "dynamic" or edge.resolution is ResolutionStatus.DYNAMIC + else "conditional" + if edge.condition + else "static", + [edge], + ) ) for node in sorted(files, key=lambda item: item.metadata["file"]): @@ -138,6 +150,7 @@ def execution_phases(self) -> list[dict[str, Any]]: "task_count": node.metadata["task_count"], "kind": "unreachable", "conditions": [], + "expressions": [], } ) return phases @@ -282,8 +295,28 @@ def _add_composition_edge(graph: RoleExecutionGraph, task_id: str, task: dict[st graph.add_node(GraphNode(target_id, NodeKind.EXTERNAL_ROLE, target, metadata={"role": target})) resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.UNRESOLVED_EXTERNAL else: - target_id = _resolve_task_file(resolved_target, source_file, file_ids) if not dynamic else None - resolution = ResolutionStatus.DYNAMIC if dynamic else ResolutionStatus.STATIC if target_id else ResolutionStatus.UNKNOWN + if dynamic: + candidate_ids = _resolve_dynamic_task_files(resolved_target, source_file, file_ids) + if candidate_ids: + for target_id in candidate_ids: + graph.add_edge( + GraphEdge( + kind, + task_id, + target_id, + ResolutionStatus.DYNAMIC, + source, + condition=_condition(task), + loop=_loop(task), + target_expression=target, + ) + ) + return + target_id = None + resolution = ResolutionStatus.DYNAMIC + else: + target_id = _resolve_task_file(resolved_target, source_file, file_ids) + resolution = ResolutionStatus.STATIC if target_id else ResolutionStatus.UNKNOWN graph.add_edge(GraphEdge(kind, task_id, target_id, resolution, source, condition=_condition(task), loop=_loop(task), target_expression=target or None)) @@ -296,6 +329,22 @@ def _resolve_task_file(target: str, source_file: str, file_ids: dict[str, str]) return matches[0] if len(matches) == 1 else None +def _resolve_dynamic_task_files(target: str, source_file: str, file_ids: dict[str, str]) -> list[str]: + """Find in-repo candidates for a templated include without claiming certainty.""" + pattern = re.sub(r"\{\{.*?\}\}", "*", target) + if PurePosixPath(pattern).name.startswith("*"): + return [] + candidates = [pattern.removeprefix("tasks/"), str(PurePosixPath(source_file).parent / pattern)] + return sorted( + { + node_id + for candidate in candidates + for file_name, node_id in file_ids.items() + if fnmatch(file_name, candidate) + } + ) + + def _condition(task: dict[str, Any]) -> str | None: value = task.get("when") return " and ".join(map(str, value)) if isinstance(value, list) else str(value) if value else None diff --git a/docsible/templates/role/sections/adaptive_diagrams.jinja2 b/docsible/templates/role/sections/adaptive_diagrams.jinja2 index eb4f01b..5e07683 100644 --- a/docsible/templates/role/sections/adaptive_diagrams.jinja2 +++ b/docsible/templates/role/sections/adaptive_diagrams.jinja2 @@ -96,9 +96,11 @@ all branches run in one execution; dynamic boundaries remain unresolved. {% set route_files = execution_phases or complexity_report.task_files_detail %} {% if complexity_report.category.value == 'enterprise' %} {% set route_files = route_files | rejectattr('kind', 'equalto', 'unreachable') | list %} +{% set route_files = route_files | rejectattr('kind', 'equalto', 'dynamic') | list %} {% endif %} {% for task_file_info in route_files %} {% if loop.first %}#### Entry point: {{ task_file_info.file }} +{% elif task_file_info.kind is defined and task_file_info.kind == 'dynamic' %}#### Dynamic path candidate: {{ task_file_info.file }} {% elif task_file_info.kind is defined and task_file_info.kind == 'conditional' %}#### Conditional path: {{ task_file_info.file }} {% elif task_file_info.kind is defined and task_file_info.kind == 'unreachable' %}#### Unreached static file: {{ task_file_info.file }} {% else %}#### Static continuation: {{ task_file_info.file }} @@ -107,9 +109,18 @@ Tasks: {{ task_file_info.task_count }} {% if task_file_info.kind is defined and task_file_info.kind == 'conditional' %} Condition: `{{ task_file_info.conditions | join(' and ') }}` {% endif %} +{% if task_file_info.kind is defined and task_file_info.kind == 'dynamic' %} +Dynamic boundary: `{{ task_file_info.expressions | join(' or ') }}` +{% endif %} {% endfor %} {% if complexity_report.category.value == 'enterprise' and execution_phases %} +{% set dynamic_count = execution_phases | selectattr('kind', 'equalto', 'dynamic') | list | length %} +{% if dynamic_count %} +#### Dynamic path candidates: {{ dynamic_count }} +Candidate files match templated boundaries. See the grouped execution overview for their source groups. + +{% endif %} {% set orphan_count = execution_phases | selectattr('kind', 'equalto', 'unreachable') | list | length %} {% if orphan_count %} #### Unreached static files: {{ orphan_count }} From f09bf6a0e2af1c2c2343ac382878971fb3bf8cfb Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 03:31:36 +0200 Subject: [PATCH 09/20] analyze role --output-format json now includes: - complexity: existing structural metrics plus: - collection_dependencies - conditional_decision_points - static reachable task files - dynamic/unknown boundaries - loop tasks - notification edges - orphan task files - execution_graph: complete serialized graph nodes and edges. --- .../complexity_analyzer/analyzers/role_analyzer.py | 7 +++++++ docsible/analyzers/complexity_analyzer/models.py | 3 +++ .../document_role/orchestrators/role_orchestrator.py | 5 +++-- docsible/formatters/text/dry_run.py | 10 ++++++++++ docsible/formatters/text/json_formatter.py | 4 ++++ docsible/graphs/role_execution.py | 2 ++ 6 files changed, 29 insertions(+), 2 deletions(-) diff --git a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py index 38a2e4b..b8ead91 100644 --- a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py +++ b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py @@ -156,6 +156,7 @@ def analyze_role_complexity( # Count role dependencies (from meta/main.yml) role_dependencies = len(role_info.get("meta", {}).get("dependencies", [])) + collection_dependencies = len(role_info.get("meta", {}).get("collections", [])) # Count role includes (include_role, import_role) role_includes = sum( @@ -233,6 +234,10 @@ def analyze_role_complexity( for edge in execution_graph.edges ), "orphan_task_files": sum(phase["kind"] == "unreachable" for phase in phases), + "conditional_decision_points": sum( + node.kind is NodeKind.TASK and "condition" in node.metadata + for node in execution_graph.nodes.values() + ), } metrics = ComplexityMetrics( total_tasks=total_tasks, @@ -241,6 +246,7 @@ def analyze_role_complexity( conditional_tasks=conditional_tasks, error_handlers=error_handlers, role_dependencies=role_dependencies, + collection_dependencies=collection_dependencies, role_includes=role_includes, task_includes=task_includes, external_integrations=len(integration_points), @@ -306,6 +312,7 @@ def analyze_role_complexity( integration_points=integration_points, recommendations=recommendations, task_files_detail=task_files_detail, + execution_graph=execution_graph.to_dict(), pattern_analysis=pattern_report, ) diff --git a/docsible/analyzers/complexity_analyzer/models.py b/docsible/analyzers/complexity_analyzer/models.py index f6270ab..11ec194 100644 --- a/docsible/analyzers/complexity_analyzer/models.py +++ b/docsible/analyzers/complexity_analyzer/models.py @@ -60,6 +60,7 @@ class ComplexityMetrics(BaseModel): # Internal composition (role orchestration) role_dependencies: int = Field(default=0, description="Role dependencies from meta/main.yml") + collection_dependencies: int = Field(default=0, description="Collection dependencies from meta/main.yml") role_includes: int = Field(default=0, description="include_role/import_role count") task_includes: int = Field(default=0, description="include_tasks/import_tasks count") @@ -71,6 +72,7 @@ class ComplexityMetrics(BaseModel): loop_tasks: int = Field(default=0) notification_edges: int = Field(default=0) orphan_task_files: int = Field(default=0) + conditional_decision_points: int = Field(default=0) # External integrations external_integrations: int = Field( @@ -138,6 +140,7 @@ class ComplexityReport(BaseModel): integration_points: list[IntegrationPoint] = Field(default_factory=list) recommendations: list[str] = Field(default_factory=list) task_files_detail: list[dict[str, Any]] = Field(default_factory=list) + execution_graph: dict[str, Any] = Field(default_factory=dict) # Pattern analysis (optional) pattern_analysis: Any | None = Field( diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index 0529973..ba84131 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -103,7 +103,7 @@ def execute(self) -> None: suppressed = [] if recommendations or self.context.analysis.output_format == "json": - self._display_recommendations(recommendations) + self._display_recommendations(recommendations, analysis_report) # Recommendation strictness applies to documentation generation only. # Validate intent reserves strictness for markdown validation below. @@ -365,7 +365,7 @@ def _display_dry_run( click.echo(summary) - def _display_recommendations(self, recommendations: list[Recommendation]) -> None: + def _display_recommendations(self, recommendations: list[Recommendation], analysis_report=None) -> None: """Display recommendations to user""" all_recs = recommendations @@ -382,6 +382,7 @@ def _display_recommendations(self, recommendations: list[Recommendation]) -> Non role_name=role_name, truncated=False, total_count=len(all_recs), + complexity_report=analysis_report, ) click.echo(json_output) return diff --git a/docsible/formatters/text/dry_run.py b/docsible/formatters/text/dry_run.py index 279582a..ac8c4d5 100644 --- a/docsible/formatters/text/dry_run.py +++ b/docsible/formatters/text/dry_run.py @@ -113,6 +113,16 @@ def _format_complexity(self, analysis_report, role_info: dict) -> str: if handlers_count > 0: lines.append(f" Handlers: {handlers_count}") + metrics = analysis_report.metrics + lines.append(f" Collections: {metrics.collection_dependencies}") + lines.append(f" Conditional decision points: {metrics.conditional_decision_points}") + lines.append( + " Execution graph: " + f"{metrics.static_reachable_task_files} static files, " + f"{metrics.dynamic_boundaries} dynamic boundaries, " + f"{metrics.orphan_task_files} orphans" + ) + return "\n".join(lines) def _format_diagrams( diff --git a/docsible/formatters/text/json_formatter.py b/docsible/formatters/text/json_formatter.py index de3b0b9..88a378f 100644 --- a/docsible/formatters/text/json_formatter.py +++ b/docsible/formatters/text/json_formatter.py @@ -15,6 +15,7 @@ def format( role_name: str = "", truncated: bool = False, total_count: int | None = None, + complexity_report=None, ) -> str: """Return JSON string of all recommendations.""" severity_counts: Counter[str] = Counter(r.severity.value.lower() for r in recommendations) @@ -44,4 +45,7 @@ def format( }, "truncated": truncated, } + if complexity_report is not None: + payload["complexity"] = complexity_report.metrics.model_dump(mode="json") + payload["execution_graph"] = complexity_report.execution_graph return json.dumps(payload, indent=2) diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index 3fac37b..78cbf87 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -234,6 +234,8 @@ def _add_tasks(graph: RoleExecutionGraph, role_name: str, task_file: dict[str, A source = SourceLocation(f"tasks/{file_name}", line) task_id = f"task:{role_name}:{file_name}:{'.'.join(map(str, index))}" metadata = {"file": file_name, "module": _module_name(task)} + if condition := _condition(task): + metadata["condition"] = condition if loop := _loop(task): metadata["loop"] = loop graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, metadata)) From 4bd34e2010e7f313dcf87cf7eba38c1d1f7350ed Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 03:40:52 +0200 Subject: [PATCH 10/20] feat: prioritize execution graph insights in generated readmes Standard README order is now: Overview Architecture Overview Execution Graph Summary Execution Routes Recommendations Variable Reference Task File Reference Handlers --- .../analyzers/recommendations/__init__.py | 28 ++++++++++++++++++- .../orchestrators/role_orchestrator.py | 2 +- .../role/sections/adaptive_diagrams.jinja2 | 10 +++++++ .../templates/role/standard_modular.jinja2 | 16 +++++++---- 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/docsible/analyzers/recommendations/__init__.py b/docsible/analyzers/recommendations/__init__.py index 3c8c5a9..2624f80 100644 --- a/docsible/analyzers/recommendations/__init__.py +++ b/docsible/analyzers/recommendations/__init__.py @@ -1,13 +1,14 @@ from pathlib import Path from docsible.models.recommendation import Recommendation +from docsible.models.severity import Severity from .enhancement import EnhancementRecommendationGenerator from .quality import QualityRecommendationGenerator from .security import SecurityRecommendationGenerator -def generate_all_recommendations(role_path: Path) -> list[Recommendation]: +def generate_all_recommendations(role_path: Path, analysis_report=None) -> list[Recommendation]: """Generate all recommendations for a role. Args: @@ -30,6 +31,31 @@ def generate_all_recommendations(role_path: Path) -> list[Recommendation]: enhancement_gen = EnhancementRecommendationGenerator() all_recommendations.extend(enhancement_gen.analyze_role(role_path)) + if analysis_report is not None: + metrics = analysis_report.metrics + if metrics.dynamic_boundaries: + all_recommendations.append( + Recommendation( + severity=Severity.INFO, + category="execution_graph", + message=f"{metrics.dynamic_boundaries} dynamic execution boundaries need runtime review", + rationale="Templated includes cannot be resolved to one static execution path.", + remediation="Review the Execution Graph Summary and validate each dynamic path.", + confidence=1.0, + ) + ) + if metrics.collection_dependencies: + all_recommendations.append( + Recommendation( + severity=Severity.INFO, + category="execution_graph", + message=f"Role depends on {metrics.collection_dependencies} Ansible collections", + rationale="Collection availability affects portability and runtime module resolution.", + remediation="Pin and document collection requirements.", + confidence=1.0, + ) + ) + # Sort by severity (critical first) all_recommendations.sort(key=lambda r: r.severity.priority, reverse=True) diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index ba84131..e8a6fa4 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -85,7 +85,7 @@ def execute(self) -> None: self._validate_documentation(role_info, analysis_report, diagrams, dependency_data) # Step 7.5: Generate recommendations (use validated role_path from step 1) - recommendations = generate_all_recommendations(role_path) + recommendations = generate_all_recommendations(role_path, analysis_report) if self.context.analysis.apply_suppressions: from docsible.suppression.engine import apply_suppressions diff --git a/docsible/templates/role/sections/adaptive_diagrams.jinja2 b/docsible/templates/role/sections/adaptive_diagrams.jinja2 index 5e07683..6f12dca 100644 --- a/docsible/templates/role/sections/adaptive_diagrams.jinja2 +++ b/docsible/templates/role/sections/adaptive_diagrams.jinja2 @@ -61,6 +61,16 @@ This role contains **{{ complexity_report.metrics.total_tasks }} tasks** across - Task Includes: {{ complexity_report.metrics.task_includes }} - Role Dependencies: {{ complexity_report.metrics.role_dependencies }} +### Execution Graph Summary +- Collection dependencies: {{ complexity_report.metrics.collection_dependencies }} +- Conditional decision points: {{ complexity_report.metrics.conditional_decision_points }} +- Statically reachable task files: {{ complexity_report.metrics.static_reachable_task_files }} +- Dynamic boundaries: {{ complexity_report.metrics.dynamic_boundaries }} +- Unknown boundaries: {{ complexity_report.metrics.unknown_boundaries }} +- Handler notification edges: {{ complexity_report.metrics.notification_edges }} +- Loop-bearing tasks: {{ complexity_report.metrics.loop_tasks }} +- Orphan task files: {{ complexity_report.metrics.orphan_task_files }} + {% if architecture_diagram %} ### Component Architecture This diagram shows the internal structure and data flow of the role: diff --git a/docsible/templates/role/standard_modular.jinja2 b/docsible/templates/role/standard_modular.jinja2 index 9b69303..26f1d8f 100644 --- a/docsible/templates/role/standard_modular.jinja2 +++ b/docsible/templates/role/standard_modular.jinja2 @@ -13,6 +13,15 @@ {% include 'sections/argument_specs.jinja2' %} {% endif %} {% endif %} +{% if not no_diagrams %} +{% include 'sections/complexity_analysis.jinja2' %} +{% include 'sections/simplification_suggestions.jinja2' %} +{% include 'sections/adaptive_diagrams.jinja2' %} +{% include 'sections/integration_boundary.jinja2' %} +{% endif %} +{% if role.defaults or role.vars %} +## Variable Reference +{% endif %} {% if role.defaults %} {% include 'sections/defaults.jinja2' %} {% endif %} @@ -20,17 +29,12 @@ {% include 'sections/vars.jinja2' %} {% endif %} {% if not no_tasks %} +## Task File Reference {% include 'sections/tasks.jinja2' %} {% endif %} {% if not no_handlers %} {% include 'sections/handlers.jinja2' %} {% endif %} -{% if not no_diagrams %} -{% include 'sections/complexity_analysis.jinja2' %} -{% include 'sections/simplification_suggestions.jinja2' %} -{% include 'sections/adaptive_diagrams.jinja2' %} -{% include 'sections/integration_boundary.jinja2' %} -{% endif %} {% include 'sections/playbook.jinja2' %} {% include 'sections/dependencies.jinja2' %} From 8414893e04916d65b48f2bb7e6683fd9e6d9400e Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 04:12:53 +0200 Subject: [PATCH 11/20] =?UTF-8?q?fix:=20first=20time=20using=20collection?= =?UTF-8?q?=20docsible/templates/collection/sections/overview.jinja2=20?= =?UTF-8?q?=E2=80=94=20removed=20a=20literal=20\n=20typo=20printed=20after?= =?UTF-8?q?=20every=20author=20name;=20fixed=20loop=20whitespace=20control?= =?UTF-8?q?.=20docsible/templates/collection/sections/galaxy=5Finfo.jinja2?= =?UTF-8?q?,=20dependencies.jinja2,=20plugin=5Flist.jinja2=20=E2=80=94=20a?= =?UTF-8?q?dded=20missing=20-%}/{%-=20trims=20so=20list=20items=20and=20co?= =?UTF-8?q?nditional=20metadata=20blocks=20don't=20leak=20blank=20lines.?= =?UTF-8?q?=20docsible/templates/collection/macros/repo=5Flinks.jinja2=20?= =?UTF-8?q?=E2=80=94=20rewrote=20render=5Farguments=5Flist=20(the=20argume?= =?UTF-8?q?nt-spec=20nested=20list=20renderer)=20with=20correct=20whitespa?= =?UTF-8?q?ce=20control;=20this=20was=20the=20single=20worst=20offender=20?= =?UTF-8?q?(4+=20blank=20lines=20per=20option).=20docsible/templates/colle?= =?UTF-8?q?ction/sections/roles=5Flist.jinja2=20=E2=80=94=20fixed=20the=20?= =?UTF-8?q?##=20Roles=20heading=20list=20and=20per-role=20sections=20to=20?= =?UTF-8?q?stop=20emitting=20a=20blank=20line=20between=20every=20entry.?= =?UTF-8?q?=20docsible/renderers/readme=5Frenderer.py=20=E2=80=94=20render?= =?UTF-8?q?=5Fcollection()=20now=20runs=20MarkdownProcessor.process(),=20s?= =?UTF-8?q?ame=20as=20render=5Frole(),=20capping=20any=20residual=20blank-?= =?UTF-8?q?line=20runs=20at=202.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docsible/commands/document_collection.py | 13 ++ .../document_role/core_orchestrated.py | 1 + docsible/renderers/readme_renderer.py | 7 +- .../collection/macros/repo_links.jinja2 | 80 ++++--- .../collection/sections/dependencies.jinja2 | 4 +- .../collection/sections/galaxy_info.jinja2 | 6 +- .../collection/sections/overview.jinja2 | 7 +- .../collection/sections/plugin_list.jinja2 | 17 +- .../collection/sections/roles_list.jinja2 | 20 +- .../role/sections/argument_specs.jinja2 | 14 ++ docsible/utils/yaml/loader.py | 21 +- tests/commands/test_document_collection.py | 215 ++++++++++++++++++ 12 files changed, 329 insertions(+), 76 deletions(-) create mode 100644 docsible/templates/role/sections/argument_specs.jinja2 create mode 100644 tests/commands/test_document_collection.py diff --git a/docsible/commands/document_collection.py b/docsible/commands/document_collection.py index 2374dc7..eec180f 100644 --- a/docsible/commands/document_collection.py +++ b/docsible/commands/document_collection.py @@ -4,6 +4,7 @@ import os from pathlib import Path +import click import yaml from docsible.commands.role_info_loader import RoleInfoLoader @@ -39,6 +40,7 @@ def document_collection_roles( repository_url: str, repo_type: str, repo_branch: str, + dry_run: bool = False, ) -> None: """Document all roles in an Ansible collection. @@ -69,6 +71,7 @@ def document_collection_roles( repository_url: Repository URL repo_type: Repository type (github, gitlab, gitea) repo_branch: Repository branch name + dry_run: Print the collection documentation plan without writing files """ collection_path_obj = Path(collection_path) @@ -100,6 +103,16 @@ def document_collection_roles( logger.warning(f"No collection marker files (galaxy.yml/yaml) found in {collection_path}") return + if dry_run: + role_count = sum( + 1 + for marker in collection_markers + for role_path in ProjectStructure(str(marker.parent)).get_roles_dir().iterdir() + if role_path.is_dir() + ) + click.echo(f"Dry-run: would document {role_count} role(s) in {collection_path}") + return + # Process each collection found for galaxy_path in collection_markers: collection_root = galaxy_path.parent diff --git a/docsible/commands/document_role/core_orchestrated.py b/docsible/commands/document_role/core_orchestrated.py index e963f53..bd293e3 100644 --- a/docsible/commands/document_role/core_orchestrated.py +++ b/docsible/commands/document_role/core_orchestrated.py @@ -432,6 +432,7 @@ def doc_the_role(**kwargs: Any) -> None: repository_url=context.repository.repository_url or "", repo_type=context.repository.repo_type or "", repo_branch=context.repository.repo_branch or "", + dry_run=context.processing.dry_run, ) except CollectionNotFoundError as e: raise click.ClickException(str(e)) from e diff --git a/docsible/renderers/readme_renderer.py b/docsible/renderers/readme_renderer.py index 0691420..7bbb685 100644 --- a/docsible/renderers/readme_renderer.py +++ b/docsible/renderers/readme_renderer.py @@ -269,8 +269,11 @@ def render_collection( # Step 4: Add Docsible tags new_content = self.tag_processor.add_tags(new_content) - # Step 5: Merge with existing content + # Step 5: Normalize excessive blank lines (same as role READMEs) + new_content = self.markdown_processor.process(new_content) + + # Step 6: Merge with existing content final_content = self.content_merger.merge(output_path, new_content, append) - # Step 6: Write file + # Step 7: Write file self.file_writer.write(output_path, final_content) diff --git a/docsible/templates/collection/macros/repo_links.jinja2 b/docsible/templates/collection/macros/repo_links.jinja2 index 2a4f07b..576e893 100644 --- a/docsible/templates/collection/macros/repo_links.jinja2 +++ b/docsible/templates/collection/macros/repo_links.jinja2 @@ -20,11 +20,7 @@ {% macro render_repo_link(repo, role_name, file_path, line, repo_type, branch) -%} {%- if repo and file_path and line is not none -%} - {%- if role.belongs_to_collection -%} - {%- set full_path = 'roles/' ~ role_name ~ '/' ~ file_path -%} - {%- else -%} - {%- set full_path = file_path -%} - {%- endif %} + {%- set full_path = 'roles/' ~ role_name ~ '/' ~ file_path -%} {%- set encoded_path = full_path | replace(' ', '%20') -%} {%- if repo_type == 'github' -%} {{ repo }}/blob/{{ branch }}/{{ encoded_path }}#L{{ line }} @@ -40,41 +36,41 @@ {%- endif %} {%- endmacro %} -{% macro render_arguments_list(arguments, level=0) %} -{% for arg, details in arguments.items() %} - {%- set indent = ' ' * level %} - {{ indent }}- **{{ arg }}** - {{ indent }} - **Required**: {{ details.required | default('false') }} - {{ indent }} - **Type**: {{ details.type }} - {{ indent }} - **Default**: {{ details.default | default('none') }} - {% if details.description is iterable and (details.description is not string and details.description is not mapping) -%} - {{ indent }} - **Description**: - {% for details_desc in details.description -%} - {{ indent }} - {{ details_desc }} - {% endfor %} - {% else %} - {{ indent }} - **Description**: {{ details.description | default('No description provided') }} - {% endif %} - {% if details.choices is defined %} - {{ indent }} - **Choices**: - {% for choice in details.choices %} - {{ indent }} - {{ choice }} - {% endfor %} - {% endif %} - {% if details.aliases is defined %} - {{ indent }} - **Aliases**: - {% for alias in details.aliases %} - {{ indent }} - {{ alias }} - {% endfor %} - {% endif %} - {% if details.type == 'dict' and details.options %} - {{ render_arguments_list(details.options, level + 1) }} - {% elif details.type == 'list' and details.elements == 'dict' %} - {% for elem in details.default %} - {% if elem is mapping %} - {{ render_arguments_list(elem, level + 1) }} - {% endif %} - {% endfor %} - {% endif %} -{% endfor %} +{% macro render_arguments_list(arguments, level=0) -%} +{%- set indent = ' ' * level -%} +{%- for arg, details in arguments.items() %} +{{ indent }}- **{{ arg }}** +{{ indent }} - **Required**: {{ details.required | default('false') }} +{{ indent }} - **Type**: {{ details.type }} +{{ indent }} - **Default**: {{ details.default | default('none') }} +{%- if details.description is iterable and (details.description is not string and details.description is not mapping) %} +{{ indent }} - **Description**: +{%- for details_desc in details.description %} +{{ indent }} - {{ details_desc }} +{%- endfor %} +{%- else %} +{{ indent }} - **Description**: {{ details.description | default('No description provided') }} +{%- endif %} +{%- if details.choices is defined %} +{{ indent }} - **Choices**: +{%- for choice in details.choices %} +{{ indent }} - {{ choice }} +{%- endfor %} +{%- endif %} +{%- if details.aliases is defined %} +{{ indent }} - **Aliases**: +{%- for alias in details.aliases %} +{{ indent }} - {{ alias }} +{%- endfor %} +{%- endif %} +{%- if details.type == 'dict' and details.options %} +{{ render_arguments_list(details.options, level + 1) }} +{%- elif details.type == 'list' and details.elements == 'dict' %} +{%- for elem in details.default %} +{%- if elem is mapping %} +{{ render_arguments_list(elem, level + 1) }} +{%- endif %} +{%- endfor %} +{%- endif %} +{% endfor -%} {% endmacro %} diff --git a/docsible/templates/collection/sections/dependencies.jinja2 b/docsible/templates/collection/sections/dependencies.jinja2 index 35bab34..0aeab7e 100644 --- a/docsible/templates/collection/sections/dependencies.jinja2 +++ b/docsible/templates/collection/sections/dependencies.jinja2 @@ -5,7 +5,7 @@ This collection depends on the following collections: -{% for dep_key, dep_value in collection.dependencies.items() %} +{% for dep_key, dep_value in collection.dependencies.items() -%} - `{{ dep_key }}`: {{ dep_value }} {% endfor %} -{% endif %} +{%- endif %} diff --git a/docsible/templates/collection/sections/galaxy_info.jinja2 b/docsible/templates/collection/sections/galaxy_info.jinja2 index 4bb383f..3f936b5 100644 --- a/docsible/templates/collection/sections/galaxy_info.jinja2 +++ b/docsible/templates/collection/sections/galaxy_info.jinja2 @@ -4,13 +4,13 @@ {% if collection.repository -%} - **Repository**: [Repository]({{ collection.repository }}) -{% endif %} +{% endif -%} {% if collection.documentation -%} - **Documentation**: [Documentation]({{ collection.documentation }}) -{% endif %} +{% endif -%} {% if collection.homepage -%} - **Homepage**: [Homepage]({{ collection.homepage }}) -{% endif %} +{% endif -%} {% if collection.issues -%} - **Issues**: [Issues]({{ collection.issues }}) {% endif %} diff --git a/docsible/templates/collection/sections/overview.jinja2 b/docsible/templates/collection/sections/overview.jinja2 index c170f1e..f049c28 100644 --- a/docsible/templates/collection/sections/overview.jinja2 +++ b/docsible/templates/collection/sections/overview.jinja2 @@ -10,10 +10,9 @@ **Version**: {{ collection.version }} **Authors**: -{% for author in collection.authors %} -- {{ author }}\n -{%- endfor %} - +{% for author in collection.authors -%} +- {{ author }} +{% endfor %} {% if collection.description -%} ## Description diff --git a/docsible/templates/collection/sections/plugin_list.jinja2 b/docsible/templates/collection/sections/plugin_list.jinja2 index a0e3bed..784420a 100644 --- a/docsible/templates/collection/sections/plugin_list.jinja2 +++ b/docsible/templates/collection/sections/plugin_list.jinja2 @@ -6,31 +6,28 @@ {% if collection.plugins.modules and collection.plugins.modules|length > 0 %} ### Modules -{% for module in collection.plugins.modules|sort %} +{% for module in collection.plugins.modules|sort -%} - `{{ module }}` {% endfor %} -{% endif %} - +{% endif -%} {% if collection.plugins.filters and collection.plugins.filters|length > 0 %} ### Filters -{% for filter in collection.plugins.filters|sort %} +{% for filter in collection.plugins.filters|sort -%} - `{{ filter }}` {% endfor %} -{% endif %} - +{% endif -%} {% if collection.plugins.lookup and collection.plugins.lookup|length > 0 %} ### Lookup Plugins -{% for lookup in collection.plugins.lookup|sort %} +{% for lookup in collection.plugins.lookup|sort -%} - `{{ lookup }}` {% endfor %} -{% endif %} - +{% endif -%} {% if collection.plugins.callback and collection.plugins.callback|length > 0 %} ### Callback Plugins -{% for callback in collection.plugins.callback|sort %} +{% for callback in collection.plugins.callback|sort -%} - `{{ callback }}` {% endfor %} {% endif %} diff --git a/docsible/templates/collection/sections/roles_list.jinja2 b/docsible/templates/collection/sections/roles_list.jinja2 index 712909f..6b347a2 100644 --- a/docsible/templates/collection/sections/roles_list.jinja2 +++ b/docsible/templates/collection/sections/roles_list.jinja2 @@ -2,31 +2,29 @@ {% from 'collection/macros/repo_links.jinja2' import render_repo_role_readme_link, render_repo_link, render_arguments_list %} ## Roles -{% for role in roles|sort(attribute='name') %} +{% for role in roles|sort(attribute='name') -%} ### [{{ role.name }}]({{ render_repo_role_readme_link(collection.repository, role.name, collection.repository_type, collection.repository_branch) }}) {% endfor %} - ## Roles vars -{% for role in roles|sort(attribute='name') %} +{% for role in roles|sort(attribute='name') -%} # [{{ role.name }}]({{ render_repo_role_readme_link(collection.repository, role.name, collection.repository_type, collection.repository_branch) }}) {% if role.meta and role.meta.galaxy_info -%} ## {{ role.name }} Description: {{ role.meta.galaxy_info.description or 'Not available.' }} -{% else %} +{%- else %} Description: Not available. {%- endif %} -{% if not no_vars %} -{% if role.argument_specs %} +{% if not no_vars -%} +{% if role.argument_specs -%}
-> {{ role.name }} Argument Specifications in meta/argument_specs -{% for section, specs in role.argument_specs.argument_specs.items() %} +> {{ role.name }} Argument Specifications in meta/argument_specs +{% for section, specs in role.argument_specs.argument_specs.items() -%} #### Key: {{ section }} **Description**: {{ specs.description or specs.short_description or 'No description provided' }} {{ render_arguments_list(specs.options) }} {% endfor %}
-{% else %} {% endif %} {% if role.defaults|length > 0 -%} @@ -116,5 +114,5 @@ Description: Not available. {%- endfor %} {%- else %} {%- endif %} -{% endif %} -{% endfor %} +{%- endif %} +{% endfor -%} diff --git a/docsible/templates/role/sections/argument_specs.jinja2 b/docsible/templates/role/sections/argument_specs.jinja2 new file mode 100644 index 0000000..b99b430 --- /dev/null +++ b/docsible/templates/role/sections/argument_specs.jinja2 @@ -0,0 +1,14 @@ +{% if role.argument_specs and role.argument_specs.argument_specs %} +### Argument Specifications +{% for spec_name, spec_data in role.argument_specs.argument_specs.items() %} +#### {{ spec_name }} +{% if spec_data.short_description %}{{ spec_data.short_description }}{% endif %} +{% if spec_data.options %} +| Parameter | Type | Required | Default | Description | +|-----------|------|----------|---------|-------------| +{% for option_name, option_data in spec_data.options.items() -%} +| `{{ option_name | escape_table_cell }}` | {{ option_data.type | default('str') | escape_table_cell }} | {{ 'Yes' if option_data.required else 'No' }} | {{ option_data.default | escape_table_value if option_data.default is defined else '-' }} | {{ option_data.description | escape_table_cell | default('') }} | +{% endfor %} +{% endif %} +{% endfor %} +{% endif %} diff --git a/docsible/utils/yaml/loader.py b/docsible/utils/yaml/loader.py index ccef1d3..0c774a5 100644 --- a/docsible/utils/yaml/loader.py +++ b/docsible/utils/yaml/loader.py @@ -19,6 +19,23 @@ logger = logging.getLogger(__name__) +class DocsibleSafeLoader(yaml.SafeLoader): + """Safe YAML loader that preserves Ansible's scalar !unsafe values.""" + + +def _construct_unsafe(loader: yaml.SafeLoader, node: yaml.Node) -> Any: + if isinstance(node, yaml.ScalarNode): + return loader.construct_scalar(node) + if isinstance(node, yaml.SequenceNode): + return loader.construct_sequence(node) + if isinstance(node, yaml.MappingNode): + return loader.construct_mapping(node) + return loader.construct_object(node) + + +DocsibleSafeLoader.add_constructor("!unsafe", _construct_unsafe) + + @cache_by_file_mtime def load_yaml_generic(filepath: str | Path) -> dict[str, Any] | None: """Load YAML file and return parsed data. @@ -36,7 +53,7 @@ def load_yaml_generic(filepath: str | Path) -> dict[str, Any] | None: """ try: with open(filepath, encoding="utf-8") as f: - data = yaml.safe_load(f) + data = yaml.load(f, Loader=DocsibleSafeLoader) return cast(dict[str, Any] | None, data) except (FileNotFoundError, yaml.YAMLError, OSError) as e: logger.error(f"Error loading {filepath}: {e}") @@ -132,7 +149,7 @@ def _read_and_parse_yaml(file_path: str) -> tuple[list[str], dict[str, Any] | No lines = file.readlines() with open(file_path, encoding="utf-8") as file: - data = yaml.safe_load(file) + data = yaml.load(file, Loader=DocsibleSafeLoader) return lines, data diff --git a/tests/commands/test_document_collection.py b/tests/commands/test_document_collection.py new file mode 100644 index 0000000..2bb4916 --- /dev/null +++ b/tests/commands/test_document_collection.py @@ -0,0 +1,215 @@ +"""Regression tests for `docsible.commands.document_collection`. + +These cover three defects found during the first full evaluation against a +real Ansible collection (prometheus-community/ansible): + +1. `document role --collection ... --dry-run` was not read-only: it created + backup files and could still crash before the actual dry-run short-circuit + existed. +2. `meta/argument_specs.yml` files using Ansible's `!unsafe` YAML tag failed + to load (`could not determine a constructor for the tag '!unsafe'`). +3. The collection-level README template referenced `role.belongs_to_collection` + in a macro where `role` was never in scope, raising + `jinja2.exceptions.UndefinedError: 'role' is undefined` for any collection + with a detectable repository URL. This also produced malformed + argument-spec tables (blank lines between rows) once the template gained + an argument-specs section. +""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +from docsible.commands.document_collection import document_collection_roles +from docsible.utils.yaml.loader import load_yaml_generic + +FIXTURES = Path(__file__).parent.parent / "fixtures" +MINIMAL_COLLECTION = FIXTURES / "minimal_collection" +MULTI_ROLE_COLLECTION = FIXTURES / "multi_role_collection" + + +def _copy_collection(src: Path, dest: Path) -> Path: + shutil.copytree(src, dest) + return dest + + +def _document( + collection_path: Path, + *, + dry_run: bool = False, + repository_url: str = "", + repo_type: str = "", + repo_branch: str = "", +) -> None: + document_collection_roles( + collection_path=str(collection_path), + playbook=None, + graph=False, + no_backup=True, + no_docsible=True, + comments=False, + task_line=False, + md_collection_template=None, + md_role_template=None, + hybrid=False, + no_vars=False, + no_tasks=False, + no_diagrams=True, + simplify_diagrams=False, + no_examples=False, + no_metadata=False, + no_handlers=False, + minimal=False, + append=False, + output="README.md", + repository_url=repository_url, + repo_type=repo_type, + repo_branch=repo_branch, + dry_run=dry_run, + ) + + +class TestCollectionDryRun: + def test_dry_run_does_not_write_any_files(self, tmp_path, capsys): + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + before = sorted(p.relative_to(collection) for p in collection.rglob("*") if p.is_file()) + + _document(collection, dry_run=True) + + after = sorted(p.relative_to(collection) for p in collection.rglob("*") if p.is_file()) + assert before == after, "dry-run must not create, modify, or remove any files" + + captured = capsys.readouterr() + assert "Dry-run" in captured.out + assert "3 role(s)" in captured.out # cache_role, db_role, proxy_role + + def test_dry_run_reports_role_count_for_single_role_collection(self, tmp_path, capsys): + collection = _copy_collection(MINIMAL_COLLECTION, tmp_path / "collection") + + _document(collection, dry_run=True) + + captured = capsys.readouterr() + assert "1 role(s)" in captured.out + + +class TestUnsafeYamlTag: + def test_load_yaml_generic_handles_unsafe_scalar(self, tmp_path): + yaml_file = tmp_path / "argument_specs.yml" + yaml_file.write_text( + "argument_specs:\n" + " main:\n" + " options:\n" + " my_var:\n" + " type: str\n" + " default: !unsafe 'plain value with {{ jinja }}'\n" + ) + + data = load_yaml_generic(yaml_file) + + assert data is not None + default = data["argument_specs"]["main"]["options"]["my_var"]["default"] + assert default == "plain value with {{ jinja }}" + + def test_collection_document_survives_unsafe_tagged_argument_specs(self, tmp_path): + collection = _copy_collection(MINIMAL_COLLECTION, tmp_path / "collection") + meta_dir = collection / "roles" / "web_role" / "meta" + meta_dir.mkdir(parents=True, exist_ok=True) + (meta_dir / "argument_specs.yml").write_text( + "argument_specs:\n" + " main:\n" + " short_description: Web role\n" + " options:\n" + " web_port:\n" + " type: int\n" + " default: !unsafe 80\n" + ) + + # Must not raise. + _document(collection, dry_run=False) + + readme = (collection / "roles" / "web_role" / "README.md").read_text() + assert "web_port" in readme + + +class TestArgumentSpecTableRendering: + def test_argument_spec_table_has_no_blank_lines_between_rows(self, tmp_path): + collection = _copy_collection(MINIMAL_COLLECTION, tmp_path / "collection") + meta_dir = collection / "roles" / "web_role" / "meta" + meta_dir.mkdir(parents=True, exist_ok=True) + (meta_dir / "argument_specs.yml").write_text( + "argument_specs:\n" + " main:\n" + " short_description: Web role\n" + " options:\n" + " web_port:\n" + " type: int\n" + " default: 80\n" + " description: Port to listen on\n" + " web_user:\n" + " type: str\n" + " default: www-data\n" + " description: User to run as\n" + ) + + _document(collection, dry_run=False) + + readme = (collection / "roles" / "web_role" / "README.md").read_text() + lines = readme.splitlines() + header_idx = next(i for i, line in enumerate(lines) if line.startswith("| Parameter")) + separator_idx = header_idx + 1 + assert lines[separator_idx].startswith("|-") + + first_row = lines[separator_idx + 1] + second_row = lines[separator_idx + 2] + # No blank line was inserted between the separator and the first row, + # or between consecutive rows. + assert first_row.strip() != "" + assert first_row.startswith("| `web_") + assert second_row.startswith("| `web_") + + +class TestCollectionReadmeGeneration: + def test_collection_readme_does_not_raise_undefined_error(self, tmp_path): + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + + # A truthy repository_url is required to exercise the buggy + # render_repo_link() path (it short-circuits to a relative link + # when no repository is known). + _document( + collection, + dry_run=False, + repository_url="https://github.com/example/repo", + repo_type="github", + repo_branch="main", + ) + + collection_readme = (collection / "README.md").read_text() + assert "roles/cache_role" in collection_readme + assert "roles/db_role" in collection_readme + assert "roles/proxy_role" in collection_readme + + def test_collection_readme_has_no_excessive_blank_lines(self, tmp_path): + """Regression test: collection templates previously left runs of many + blank lines between roles, list items, and argument-spec entries, + making generated collection READMEs unreadable. render_collection + must apply the same blank-line normalization as render_role. + """ + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + + _document(collection, dry_run=False) + + readme = (collection / "README.md").read_text() + blank_run = 0 + max_blank_run = 0 + for line in readme.splitlines(): + if line.strip() == "": + blank_run += 1 + max_blank_run = max(max_blank_run, blank_run) + else: + blank_run = 0 + + assert max_blank_run <= 2, ( + f"found a run of {max_blank_run} consecutive blank lines in the " + "generated collection README" + ) From 651cdc32ba3132cbe8e6d75d524770b5973cc7f9 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 04:54:00 +0200 Subject: [PATCH 12/20] =?UTF-8?q?feat:=20conolidation=20role=20analysis=20?= =?UTF-8?q?for=20role=20and=20colleciton=20and=20complexity=20overview=20f?= =?UTF-8?q?or=20collections=20Complexity=20Overview:=20total=20roles,=20to?= =?UTF-8?q?tal=20tasks,=20and=20a=20category=20breakdown=20table=20(Enterp?= =?UTF-8?q?rise/Complex/Medium/Simple=20counts)=20=E2=80=94=20factual=20ag?= =?UTF-8?q?gregates,=20not=20a=20synthesized=20"collection=20complexity=20?= =?UTF-8?q?score."=20Role=20Index:=20one=20row=20per=20role=20=E2=80=94=20?= =?UTF-8?q?name=20(linked),=20complexity=20badge,=20task=20count,=20critic?= =?UTF-8?q?al/warning=20counts,=20top=20finding=20=E2=80=94=20sorted=20by?= =?UTF-8?q?=20complexity=20descending=20(ties=20broken=20alphabetically),?= =?UTF-8?q?=20so=20the=20roles=20needing=20attention=20surface=20first=20w?= =?UTF-8?q?ithout=20opening=20anything.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docsible/commands/document_collection.py | 55 +++++- .../orchestrators/role_orchestrator.py | 35 ++-- .../commands/document_role/role_analysis.py | 181 ++++++++++++++++++ docsible/commands/scan/collection.py | 19 +- docsible/templates/collection/main.jinja2 | 2 + .../sections/complexity_overview.jinja2 | 29 +++ .../orchestrators/test_role_orchestrator.py | 20 +- tests/commands/document_role/test_fail_on.py | 8 +- .../document_role/test_role_analysis.py | 138 +++++++++++++ tests/commands/scan/test_scan_collection.py | 38 +++- tests/commands/test_document_collection.py | 75 +++++++- tests/help/test_cli_integration.py | 18 +- tests/phase3/test_intent_behavior.py | 8 +- tests/phase3/test_wizard.py | 2 +- 14 files changed, 567 insertions(+), 61 deletions(-) create mode 100644 docsible/commands/document_role/role_analysis.py create mode 100644 docsible/templates/collection/sections/complexity_overview.jinja2 create mode 100644 tests/commands/document_role/test_role_analysis.py diff --git a/docsible/commands/document_collection.py b/docsible/commands/document_collection.py index eec180f..768d44a 100644 --- a/docsible/commands/document_collection.py +++ b/docsible/commands/document_collection.py @@ -7,6 +7,7 @@ import click import yaml +from docsible.commands.document_role.role_analysis import analyze_role, render_analyzed_role from docsible.commands.role_info_loader import RoleInfoLoader from docsible.exceptions import CollectionNotFoundError from docsible.renderers.readme_renderer import ReadmeRenderer @@ -176,26 +177,72 @@ def document_collection_roles( role_info["docsible"] = manage_docsible_file_keys(role_path / ".docsible") - renderer = ReadmeRenderer(backup=not no_backup) + # Analyze complexity, execution graph, and recommendations — + # identical to standalone `document role` and `scan collection` + # (previously this was skipped entirely for collection roles). + analysis = analyze_role(role_info, role_path, min_confidence=0.7) + role_readme_path = role_path / output template_type = "hybrid" if hybrid else "standard_modular" - renderer.render_role( + render_analyzed_role( role_info=role_info, + role_path=role_path, + analysis=analysis, output_path=role_readme_path, template_type=template_type, custom_template_path=md_role_template, + generate_graph=graph, + minimal=minimal, + simplify_diagrams=simplify_diagrams, no_vars=no_vars, no_tasks=no_tasks, no_diagrams=no_diagrams, - simplify_diagrams=simplify_diagrams, no_examples=no_examples, no_metadata=no_metadata, no_handlers=no_handlers, + include_complexity=hybrid, append=append, + backup=not no_backup, + playbook_content=playbook_content, + ) + + warning_count = sum( + 1 for r in analysis.recommendations if r.severity.value == "warning" + ) + critical_count = sum( + 1 for r in analysis.recommendations if r.severity.value == "critical" + ) + logger.info( + f"✓ Documented role: {role_name} " + f"({analysis.complexity_report.category.value}, " + f"{critical_count} critical, {warning_count} warning)" + ) + + # Summary fields for the collection-level Role Index (a human + # scanning the collection README needs to see, at a glance, + # which roles are complex/risky before opening any of them). + category = analysis.complexity_report.category.value + role_info["complexity_category"] = category + role_info["complexity_rank"] = { + "simple": 0, + "medium": 1, + "complex": 2, + "enterprise": 3, + }.get(category, 0) + role_info["complexity_badge"] = { + "simple": "🟢 SIMPLE", + "medium": "🟡 MEDIUM", + "complex": "🟠 COMPLEX", + "enterprise": "🔴 ENTERPRISE", + }.get(category, category.upper()) + role_info["complexity_task_count"] = analysis.complexity_report.metrics.total_tasks + role_info["complexity_critical_count"] = critical_count + role_info["complexity_warning_count"] = warning_count + role_info["complexity_top_finding"] = ( + analysis.recommendations[0].message if analysis.recommendations else None ) - logger.info(f"✓ Documented role: {role_name}") roles_info.append(role_info) # Generate collection README diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index e8a6fa4..6b9a6dc 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -9,8 +9,8 @@ import click -from docsible.analyzers.recommendations import generate_all_recommendations from docsible.commands.document_role.models import RoleCommandContext +from docsible.commands.document_role.role_analysis import analyze_role from docsible.commands.role_info_loader import RoleInfoLoader from docsible.formatters.text.dry_run import DryRunFormatter from docsible.models.recommendation import Recommendation @@ -55,8 +55,10 @@ def execute(self) -> None: # Step 3: Build role info role_info = self._build_role_info(role_path, playbook_content) - # Step 4: Analyze complexity - analysis_report = self._analyze_complexity(role_info) + # Step 4: Analyze complexity + recommendations (shared with + # `document role --collection` and `scan collection`) + analysis = self._analyze_role(role_info, role_path) + analysis_report = analysis.complexity_report if ( self.context.analysis.recommendations_only @@ -84,8 +86,9 @@ def execute(self) -> None: ): self._validate_documentation(role_info, analysis_report, diagrams, dependency_data) - # Step 7.5: Generate recommendations (use validated role_path from step 1) - recommendations = generate_all_recommendations(role_path, analysis_report) + # Step 7.5: Recommendations were already computed alongside complexity + # in step 4 (shared analyze_role()), using the validated role_path. + recommendations = analysis.recommendations if self.context.analysis.apply_suppressions: from docsible.suppression.engine import apply_suppressions @@ -213,31 +216,31 @@ def _build_role_info(self, role_path: Path, playbook_content: str | None) -> dic read_docsible=not self.context.processing.no_docsible, ) - def _analyze_complexity(self, role_info: dict): - """Analyze role complexity. + def _analyze_role(self, role_info: dict, role_path: Path): + """Analyze role complexity and recommendations. - Reuses cached analysis from smart defaults if available to avoid + Delegates to the shared `analyze_role()` used by `document role + --collection` and `scan collection`, so all three produce identical + complexity/execution-graph/recommendation results for a given role. + Reuses cached complexity from smart defaults if available, to avoid duplicate analysis. Args: role_info: Role information dictionary + role_path: Validated path to the role directory Returns: - Complexity analysis report + RoleAnalysis with complexity_report and recommendations """ - # Check if we have a cached report from smart defaults if self.context.analysis.cached_complexity_report: logger.debug("Reusing complexity analysis from smart defaults (avoiding duplicate)") - return self.context.analysis.cached_complexity_report - # No cached report available, perform fresh analysis - from docsible.analyzers import analyze_role_complexity - - logger.debug("Performing fresh complexity analysis") - return analyze_role_complexity( + return analyze_role( role_info, + role_path, include_patterns=self.context.analysis.simplification_report, min_confidence=0.7, + cached_complexity_report=self.context.analysis.cached_complexity_report, ) def _display_analysis_and_exit(self, analysis_report, role_info: dict) -> None: diff --git a/docsible/commands/document_role/role_analysis.py b/docsible/commands/document_role/role_analysis.py new file mode 100644 index 0000000..abb8f0c --- /dev/null +++ b/docsible/commands/document_role/role_analysis.py @@ -0,0 +1,181 @@ +"""Shared role analysis and rendering, reused across entry points. + +Three call sites need "what does this role's complexity/execution graph/ +recommendations look like": single-role `document role`, `document role +--collection`, and `scan collection`. Before this module they each computed +it independently and had drifted (e.g. `scan collection` did not pass the +complexity report into recommendation generation, so it missed the +graph-aware findings the single-role path already had). + +This module splits the work into two layers: + +- `analyze_role()` — pure computation, no file I/O. Used by all three + callers, including `scan collection` which never renders a README. +- `render_analyzed_role()` — diagram generation, dependency matrix, and the + actual `ReadmeRenderer.render_role()` call. Used by callers that write a + role README (single-role `document role`, and `document role + --collection`, which previously skipped this entirely). +""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from docsible.analyzers import analyze_role_complexity +from docsible.analyzers.complexity_analyzer.models import ComplexityReport +from docsible.analyzers.recommendations import generate_all_recommendations +from docsible.models.recommendation import Recommendation + + +@dataclass +class RoleAnalysis: + """Pure analysis result for one role: no file I/O, no rendering.""" + + complexity_report: ComplexityReport + recommendations: list[Recommendation] + + +def analyze_role( + role_info: dict[str, Any], + role_path: Path, + *, + include_patterns: bool = False, + min_confidence: float = 0.7, + cached_complexity_report: ComplexityReport | None = None, +) -> RoleAnalysis: + """Compute complexity (incl. execution graph) and recommendations. + + Args: + role_info: Role information dictionary from RoleInfoLoader + role_path: Path to the role directory (recommendations scan the + filesystem directly for some checks, e.g. vault encryption) + include_patterns: Whether to run the (expensive) pattern analyzer + min_confidence: Minimum confidence for pattern detection + cached_complexity_report: Reuse an already-computed report (e.g. from + smart defaults) instead of analyzing again + + Returns: + RoleAnalysis with the complexity report and recommendations + """ + complexity_report = cached_complexity_report or analyze_role_complexity( + role_info, + include_patterns=include_patterns, + min_confidence=min_confidence, + ) + recommendations = generate_all_recommendations(role_path, complexity_report) + return RoleAnalysis(complexity_report=complexity_report, recommendations=recommendations) + + +def render_analyzed_role( + *, + role_info: dict[str, Any], + role_path: Path, + analysis: RoleAnalysis, + output_path: Path, + template_type: str = "standard_modular", + custom_template_path: str | None = None, + generate_graph: bool = False, + minimal: bool = False, + simplify_diagrams: bool = False, + show_dependencies: bool = False, + no_vars: bool = False, + no_tasks: bool = False, + no_diagrams: bool = False, + no_examples: bool = False, + no_metadata: bool = False, + no_handlers: bool = False, + include_complexity: bool = False, + append: bool = False, + backup: bool = True, + validate: bool = True, + auto_fix: bool = False, + strict_validation: bool = False, + playbook_content: str | None = None, +) -> Path: + """Generate diagrams/dependency matrix and render a role README. + + Reuses the same diagram-generation helpers as single-role `document + role`, so a role documented as part of a collection gets an identical + Architecture Overview / Execution Graph Summary / Execution Routes / + Complexity Analysis to a standalone role. + + Returns: + The path written. + """ + from docsible.commands.document_role.helpers import ( + generate_dependency_matrix, + generate_integration_and_architecture_diagrams, + generate_mermaid_diagrams, + ) + from docsible.graphs import build_role_execution_graph + from docsible.renderers.readme_renderer import ReadmeRenderer + + analysis_report = analysis.complexity_report + execution_graph = build_role_execution_graph(role_info) + + diagrams = generate_mermaid_diagrams( + generate_graph=generate_graph, + role_info=role_info, + playbook_content=playbook_content, + analysis_report=analysis_report, + minimal=minimal, + simplify_diagrams=simplify_diagrams, + ) + diagrams["generate_graph"] = generate_graph + diagrams["execution_graph"] = execution_graph + diagrams["execution_phases"] = execution_graph.execution_phases() + + integration_boundary, architecture = generate_integration_and_architecture_diagrams( + generate_graph=generate_graph, + role_info=role_info, + analysis_report=analysis_report, + execution_graph=execution_graph, + ) + diagrams["integration_boundary_diagram"] = integration_boundary + diagrams["architecture_diagram"] = architecture + + dependency_matrix, dependency_summary, show_dependency_matrix = generate_dependency_matrix( + show_dependencies=show_dependencies, + role_info=role_info, + analysis_report=analysis_report, + ) + + render_options: dict[str, Any] = { + "mermaid_code_per_file": diagrams.get("mermaid_code_per_file", {}), + "sequence_diagram_high_level": diagrams.get("sequence_diagram_high_level"), + "sequence_diagram_detailed": diagrams.get("sequence_diagram_detailed"), + "state_diagram": diagrams.get("state_diagram"), + "integration_boundary_diagram": diagrams.get("integration_boundary_diagram"), + "architecture_diagram": diagrams.get("architecture_diagram"), + "execution_phases": diagrams.get("execution_phases"), + "complexity_report": analysis_report, + "include_complexity": include_complexity, + "dependency_matrix": dependency_matrix, + "dependency_summary": dependency_summary, + "show_dependency_matrix": show_dependency_matrix, + "no_vars": no_vars, + "no_tasks": no_tasks, + "no_diagrams": no_diagrams, + "simplify_diagrams": simplify_diagrams, + "no_examples": no_examples, + "no_metadata": no_metadata, + "no_handlers": no_handlers, + } + + renderer = ReadmeRenderer( + backup=backup, + validate=validate, + auto_fix=auto_fix, + strict_validation=strict_validation, + ) + renderer.render_role( + role_info=role_info, + output_path=output_path, + template_type=template_type, + custom_template_path=custom_template_path, + append=append, + **render_options, + ) + return output_path diff --git a/docsible/commands/scan/collection.py b/docsible/commands/scan/collection.py index 3359457..afbaf46 100644 --- a/docsible/commands/scan/collection.py +++ b/docsible/commands/scan/collection.py @@ -48,8 +48,7 @@ def _analyse_role(role_path: Path, git_info: dict) -> RoleResult: Returns: RoleResult with metrics and findings. """ - from docsible.analyzers import analyze_role_complexity - from docsible.analyzers.recommendations import generate_all_recommendations + from docsible.commands.document_role.role_analysis import analyze_role from docsible.commands.role_info_loader import RoleInfoLoader from docsible.models.severity import Severity @@ -80,16 +79,14 @@ def _analyse_role(role_path: Path, git_info: dict) -> RoleResult: ) variable_count = defaults_count + vars_count - # Complexity analysis - complexity_report = analyze_role_complexity( - role_info, - include_patterns=False, - min_confidence=0.7, - ) + # Complexity + recommendations — shared with `document role` and + # `document role --collection` so all three agree (this previously called + # generate_all_recommendations() without the complexity report, missing + # the graph-aware findings the other two paths already had). + analysis = analyze_role(role_info, role_path, min_confidence=0.7) + complexity_report = analysis.complexity_report complexity = _complexity_label(complexity_report.category.value) - - # Recommendations - recommendations = generate_all_recommendations(role_path) + recommendations = analysis.recommendations critical_count = sum(1 for r in recommendations if r.severity == Severity.CRITICAL) warning_count = sum(1 for r in recommendations if r.severity == Severity.WARNING) diff --git a/docsible/templates/collection/main.jinja2 b/docsible/templates/collection/main.jinja2 index b84158a..e07e495 100644 --- a/docsible/templates/collection/main.jinja2 +++ b/docsible/templates/collection/main.jinja2 @@ -1,6 +1,8 @@ {# Main collection template - modularized version #} {% include 'collection/sections/overview.jinja2' %} +{% include 'collection/sections/complexity_overview.jinja2' %} + {% include 'collection/sections/roles_list.jinja2' %} {% include 'collection/sections/dependencies.jinja2' %} diff --git a/docsible/templates/collection/sections/complexity_overview.jinja2 b/docsible/templates/collection/sections/complexity_overview.jinja2 new file mode 100644 index 0000000..2a5fb05 --- /dev/null +++ b/docsible/templates/collection/sections/complexity_overview.jinja2 @@ -0,0 +1,29 @@ +{# Collection-wide complexity overview: aggregate facts (not a synthesized + score, since a collection has no single execution graph the way one role + does) plus a per-role index sorted by complexity, so a reader can see at + a glance which roles need the most attention before opening any of them + individually. #} +{% from 'collection/macros/repo_links.jinja2' import render_repo_role_readme_link %} +{% if roles and roles[0].complexity_category is defined %} +{% set sorted_roles = roles|sort(attribute='name')|sort(attribute='complexity_rank', reverse=true) %} +## Complexity Overview + +This collection contains **{{ roles|length }} roles** and **{{ roles|sum(attribute='complexity_task_count') }} tasks** in total. + +| Category | Roles | +|----------|-------| +| 🔴 Enterprise | {{ roles|selectattr('complexity_category', 'equalto', 'enterprise')|list|length }} | +| 🟠 Complex | {{ roles|selectattr('complexity_category', 'equalto', 'complex')|list|length }} | +| 🟡 Medium | {{ roles|selectattr('complexity_category', 'equalto', 'medium')|list|length }} | +| 🟢 Simple | {{ roles|selectattr('complexity_category', 'equalto', 'simple')|list|length }} | + +### Role Index + +Sorted by complexity, most involved first. + +| Role | Complexity | Tasks | Critical | Warning | Top Finding | +|------|-----------|-------|----------|---------|-------------| +{% for role in sorted_roles -%} +| [{{ role.name }}]({{ render_repo_role_readme_link(collection.repository, role.name, collection.repository_type, collection.repository_branch) }}) | {{ role.complexity_badge }} | {{ role.complexity_task_count }} | {{ role.complexity_critical_count }} | {{ role.complexity_warning_count }} | {{ role.complexity_top_finding | escape_table_cell if role.complexity_top_finding else '—' }} | +{% endfor %} +{% endif %} diff --git a/tests/commands/document_role/orchestrators/test_role_orchestrator.py b/tests/commands/document_role/orchestrators/test_role_orchestrator.py index a61463c..8000797 100644 --- a/tests/commands/document_role/orchestrators/test_role_orchestrator.py +++ b/tests/commands/document_role/orchestrators/test_role_orchestrator.py @@ -107,14 +107,16 @@ def test_build_role_info(self, minimal_context, temp_role_dir): assert "name" in role_info assert role_info["name"] == "test_role" - def test_analyze_complexity(self, minimal_context, temp_role_dir): - """Test complexity analysis.""" + def test_analyze_role(self, minimal_context, temp_role_dir): + """Test complexity + recommendation analysis (shared with collection/scan).""" minimal_context.paths.role_path = temp_role_dir orchestrator = RoleOrchestrator(minimal_context) role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis = orchestrator._analyze_role(role_info, temp_role_dir) + analysis_report = analysis.complexity_report + assert analysis.recommendations is not None assert analysis_report is not None assert hasattr(analysis_report, "category") assert hasattr(analysis_report, "metrics") @@ -143,7 +145,7 @@ def test_generate_diagrams_disabled(self, minimal_context, temp_role_dir): orchestrator = RoleOrchestrator(minimal_context) role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report diagrams = orchestrator._generate_diagrams(role_info, analysis_report, None) @@ -156,7 +158,7 @@ def test_generate_diagrams_enabled(self, minimal_context, temp_role_dir): orchestrator = RoleOrchestrator(minimal_context) role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report diagrams = orchestrator._generate_diagrams(role_info, analysis_report, None) @@ -172,7 +174,7 @@ def test_generate_dependencies(self, minimal_context, temp_role_dir): orchestrator = RoleOrchestrator(minimal_context) role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report dependency_data = orchestrator._generate_dependencies(role_info, analysis_report) @@ -187,7 +189,7 @@ def test_display_dry_run(self, mock_echo, minimal_context, temp_role_dir): orchestrator = RoleOrchestrator(minimal_context) role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report diagrams = {"generate_graph": False} dependency_data = { "dependency_matrix": None, @@ -214,7 +216,7 @@ def test_render_documentation(self, mock_renderer_class, mock_echo, minimal_cont mock_renderer_class.return_value = mock_renderer role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report diagrams = { "generate_graph": False, "mermaid_code_per_file": {}, @@ -256,7 +258,7 @@ def test_render_documentation_hybrid_mode( mock_renderer_class.return_value = mock_renderer role_info = orchestrator._build_role_info(temp_role_dir, None) - analysis_report = orchestrator._analyze_complexity(role_info) + analysis_report = orchestrator._analyze_role(role_info, temp_role_dir).complexity_report diagrams = { "generate_graph": False, "mermaid_code_per_file": {}, diff --git a/tests/commands/document_role/test_fail_on.py b/tests/commands/document_role/test_fail_on.py index e6525b9..49ba344 100644 --- a/tests/commands/document_role/test_fail_on.py +++ b/tests/commands/document_role/test_fail_on.py @@ -58,10 +58,9 @@ def _make_orchestrator(context: RoleCommandContext) -> RoleOrchestrator: _PATCH_VALIDATE = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._validate_paths" _PATCH_PLAYBOOK = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._load_playbook" _PATCH_BUILD = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._build_role_info" -_PATCH_COMPLEXITY = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._analyze_complexity" +_PATCH_ANALYZE = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._analyze_role" _PATCH_DIAGRAMS = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._generate_diagrams" _PATCH_DEPS = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator._generate_dependencies" -_PATCH_RECS = "docsible.commands.document_role.orchestrators.role_orchestrator.generate_all_recommendations" _PATCH_SUPPRESSIONS = "docsible.commands.document_role.orchestrators.role_orchestrator.RoleOrchestrator" @@ -71,7 +70,7 @@ def _run_execute_with_recs(recs: list[Recommendation], context: RoleCommandConte fake_path = Path("/fake/role") fake_role_info: dict = {"name": "fake_role"} - fake_analysis = MagicMock() + fake_analysis = MagicMock(recommendations=recs) fake_diagrams: dict = { "generate_graph": False, "mermaid_code_per_file": {}, @@ -91,10 +90,9 @@ def _run_execute_with_recs(recs: list[Recommendation], context: RoleCommandConte patch(_PATCH_VALIDATE, return_value=fake_path), patch(_PATCH_PLAYBOOK, return_value=None), patch(_PATCH_BUILD, return_value=fake_role_info), - patch(_PATCH_COMPLEXITY, return_value=fake_analysis), + patch(_PATCH_ANALYZE, return_value=fake_analysis), patch(_PATCH_DIAGRAMS, return_value=fake_diagrams), patch(_PATCH_DEPS, return_value=fake_deps), - patch(_PATCH_RECS, return_value=recs), # Skip suppression machinery — just return recs unchanged patch( "docsible.suppression.engine.apply_suppressions", diff --git a/tests/commands/document_role/test_role_analysis.py b/tests/commands/document_role/test_role_analysis.py new file mode 100644 index 0000000..782b6d9 --- /dev/null +++ b/tests/commands/document_role/test_role_analysis.py @@ -0,0 +1,138 @@ +"""Tests for the shared role analysis/render pipeline. + +`analyze_role()` and `render_analyzed_role()` are the single implementation +used by single-role `document role`, `document role --collection`, and +`scan collection`. These tests guard the parity property directly: a role +analyzed/rendered through this module must produce the same +complexity/execution-graph/recommendation content regardless of which +command called it. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from docsible.commands.document_role.role_analysis import ( + RoleAnalysis, + analyze_role, + render_analyzed_role, +) +from docsible.commands.role_info_loader import RoleInfoLoader + + +def _write_role(root: Path, *, task_count: int) -> Path: + """Write a minimal role with `task_count` named, no-op tasks.""" + (root / "tasks").mkdir(parents=True) + (root / "handlers").mkdir(parents=True) + (root / "defaults").mkdir(parents=True) + + tasks = "\n".join( + f"- name: Task {i}\n debug:\n msg: 'step {i}'" for i in range(task_count) + ) + (root / "tasks" / "main.yml").write_text(f"---\n{tasks}\n") + (root / "handlers" / "main.yml").write_text( + "---\n- name: restart service\n debug:\n msg: restart\n" + ) + (root / "defaults" / "main.yml").write_text("---\nsome_var: 1\n") + return root + + +@pytest.fixture +def small_role(tmp_path) -> Path: + return _write_role(tmp_path / "small_role", task_count=3) + + +@pytest.fixture +def large_role(tmp_path) -> Path: + # 26+ tasks crosses the SIMPLE/MEDIUM -> COMPLEX threshold. + return _write_role(tmp_path / "large_role", task_count=30) + + +class TestAnalyzeRole: + def test_returns_complexity_report_and_recommendations(self, small_role): + role_info = RoleInfoLoader().load(small_role) + + analysis = analyze_role(role_info, small_role) + + assert isinstance(analysis, RoleAnalysis) + assert analysis.complexity_report is not None + assert analysis.complexity_report.metrics.total_tasks == 3 + assert isinstance(analysis.recommendations, list) + + def test_reuses_cached_complexity_report_without_recomputing(self, small_role): + role_info = RoleInfoLoader().load(small_role) + cached = analyze_role(role_info, small_role).complexity_report + + analysis = analyze_role(role_info, small_role, cached_complexity_report=cached) + + # Same object identity: the cache path must not run analysis again. + assert analysis.complexity_report is cached + + def test_large_role_is_classified_complex(self, large_role): + role_info = RoleInfoLoader().load(large_role) + + analysis = analyze_role(role_info, large_role) + + assert analysis.complexity_report.category.value in ("complex", "enterprise") + + +class TestRenderAnalyzedRole: + def test_complex_role_readme_has_architecture_and_execution_routes(self, large_role): + """Regression test for the collection-parity gap: a role analyzed and + rendered through this shared pipeline must show the same Architecture + Overview / Execution Graph Summary / Execution Routes sections that + standalone `document role` produces for a COMPLEX role. + """ + role_info = RoleInfoLoader().load(large_role) + analysis = analyze_role(role_info, large_role) + output_path = large_role / "README.md" + + render_analyzed_role( + role_info=role_info, + role_path=large_role, + analysis=analysis, + output_path=output_path, + ) + + readme = output_path.read_text() + assert "## Architecture Overview" in readme + assert "### Execution Graph Summary" in readme + assert "### Execution Routes" in readme + assert "Statically reachable task files" in readme + + def test_small_role_renders_without_error(self, small_role): + role_info = RoleInfoLoader().load(small_role) + analysis = analyze_role(role_info, small_role) + output_path = small_role / "README.md" + + render_analyzed_role( + role_info=role_info, + role_path=small_role, + analysis=analysis, + output_path=output_path, + ) + + assert output_path.exists() + + def test_no_excessive_blank_lines(self, large_role): + role_info = RoleInfoLoader().load(large_role) + analysis = analyze_role(role_info, large_role) + output_path = large_role / "README.md" + + render_analyzed_role( + role_info=role_info, + role_path=large_role, + analysis=analysis, + output_path=output_path, + ) + + blank_run = max_blank_run = 0 + for line in output_path.read_text().splitlines(): + if line.strip() == "": + blank_run += 1 + max_blank_run = max(max_blank_run, blank_run) + else: + blank_run = 0 + assert max_blank_run <= 2 diff --git a/tests/commands/scan/test_scan_collection.py b/tests/commands/scan/test_scan_collection.py index 36dd47d..a3e6220 100644 --- a/tests/commands/scan/test_scan_collection.py +++ b/tests/commands/scan/test_scan_collection.py @@ -7,8 +7,10 @@ from __future__ import annotations import json +import shutil from pathlib import Path +import click import pytest from click.testing import CliRunner @@ -23,7 +25,7 @@ MULTI_ROLE_COLLECTION = FIXTURES / "multi_role_collection" -def _invoke(*args: str) -> click.testing.Result: # type: ignore[name-defined] +def _invoke(*args: str) -> click.testing.Result: runner = CliRunner() return runner.invoke(cli, ["scan", "collection", *args], catch_exceptions=False) @@ -206,3 +208,37 @@ def test_scan_top_n_json_limits_roles_array(self): assert len(data["roles"]) <= 2, ( f"Expected <=2 roles in JSON output, got {len(data['roles'])}" ) + + +class TestScanRecommendationParity: + """Regression test: `_analyse_role()` previously called + `generate_all_recommendations(role_path)` without the complexity report, + silently missing the graph-aware findings (e.g. collection dependencies) + that `document role` already produced. It must now use the same + `analyze_role()` pipeline as `document role` / `document role + --collection`. + """ + + def test_scan_surfaces_collection_dependency_recommendation(self, tmp_path): + collection = tmp_path / "collection" + shutil.copytree(MINIMAL_COLLECTION, collection) + meta_path = collection / "roles" / "web_role" / "meta" / "main.yml" + meta_path.write_text( + "galaxy_info:\n" + " author: Test Author\n" + " description: Web role for testing\n" + " license: MIT\n" + " min_ansible_version: '2.9'\n" + "dependencies: []\n" + "collections:\n" + " - community.general\n" + ) + + result = _invoke(str(collection), "--output-format", "json") + + assert result.exit_code == 0 + data = json.loads(result.output) + role = next(r for r in data["roles"] if r["name"] == "web_role") + assert any( + "collections" in msg.lower() for msg in role["top_recommendations"] + ), role["top_recommendations"] diff --git a/tests/commands/test_document_collection.py b/tests/commands/test_document_collection.py index 2bb4916..a85adbb 100644 --- a/tests/commands/test_document_collection.py +++ b/tests/commands/test_document_collection.py @@ -41,6 +41,7 @@ def _document( repository_url: str = "", repo_type: str = "", repo_branch: str = "", + no_diagrams: bool = True, ) -> None: document_collection_roles( collection_path=str(collection_path), @@ -55,7 +56,7 @@ def _document( hybrid=False, no_vars=False, no_tasks=False, - no_diagrams=True, + no_diagrams=no_diagrams, simplify_diagrams=False, no_examples=False, no_metadata=False, @@ -213,3 +214,75 @@ def test_collection_readme_has_no_excessive_blank_lines(self, tmp_path): f"found a run of {max_blank_run} consecutive blank lines in the " "generated collection README" ) + + +class TestCollectionRoleAnalysisParity: + """Regression tests for the collection-parity gap: roles documented as + part of a collection previously skipped complexity analysis, the + execution graph, and recommendations entirely (they were only computed + by standalone `document role`). `document_collection_roles()` must now + route every role through the same `analyze_role()` / + `render_analyzed_role()` pipeline. + """ + + def test_complex_collection_role_gets_architecture_and_execution_routes(self, tmp_path): + collection = _copy_collection(MINIMAL_COLLECTION, tmp_path / "collection") + tasks_path = collection / "roles" / "web_role" / "tasks" / "main.yml" + tasks = "\n".join( + f"- name: Task {i}\n debug:\n msg: 'step {i}'" for i in range(30) + ) + tasks_path.write_text(f"---\n{tasks}\n") + + _document(collection, dry_run=False, no_diagrams=False) + + role_readme = (collection / "roles" / "web_role" / "README.md").read_text() + assert "## Architecture Overview" in role_readme + assert "### Execution Graph Summary" in role_readme + assert "### Execution Routes" in role_readme + + def test_simple_collection_role_still_renders_without_error(self, tmp_path): + collection = _copy_collection(MINIMAL_COLLECTION, tmp_path / "collection") + + _document(collection, dry_run=False, no_diagrams=False) + + role_readme_path = collection / "roles" / "web_role" / "README.md" + assert role_readme_path.exists() + + +class TestCollectionComplexityOverview: + """The collection-level README must show an at-a-glance complexity + breakdown and a per-role index sorted by complexity (most involved + first), so a reader knows which roles need attention before opening + any of them individually. + """ + + def test_collection_readme_has_complexity_overview_and_role_index(self, tmp_path): + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + + _document(collection, dry_run=False) + + readme = (collection / "README.md").read_text() + assert "## Complexity Overview" in readme + assert "### Role Index" in readme + assert "cache_role" in readme + assert "db_role" in readme + assert "proxy_role" in readme + + def test_role_index_sorts_most_complex_role_first(self, tmp_path): + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + tasks_path = collection / "roles" / "db_role" / "tasks" / "main.yml" + tasks = "\n".join( + f"- name: Task {i}\n debug:\n msg: 'step {i}'" for i in range(30) + ) + tasks_path.write_text(f"---\n{tasks}\n") + + _document(collection, dry_run=False) + + readme = (collection / "README.md").read_text() + index_start = readme.index("### Role Index") + assert readme.index("db_role", index_start) < readme.index( + "cache_role", index_start + ) + assert readme.index("db_role", index_start) < readme.index( + "proxy_role", index_start + ) diff --git a/tests/help/test_cli_integration.py b/tests/help/test_cli_integration.py index ef629fb..d4a656e 100644 --- a/tests/help/test_cli_integration.py +++ b/tests/help/test_cli_integration.py @@ -50,9 +50,9 @@ def test_role_command_uses_brief_help_command(self): def test_role_help_shows_brief_by_default(self): """Default --help should show brief help with --help-full pointer.""" - try: - from docsible.utils.cli_helpers import BriefHelpCommand - except ImportError: + import docsible.utils.cli_helpers as cli_helpers + + if not hasattr(cli_helpers, "BriefHelpCommand"): pytest.skip("BriefHelpCommand not yet implemented in cli_helpers.py") from docsible.cli import cli runner = CliRunner() @@ -63,9 +63,9 @@ def test_role_help_shows_brief_by_default(self): def test_role_full_help_shows_more_options(self): """--help-full should show all options (more than brief help).""" - try: - from docsible.utils.cli_helpers import BriefHelpCommand - except ImportError: + import docsible.utils.cli_helpers as cli_helpers + + if not hasattr(cli_helpers, "BriefHelpCommand"): pytest.skip("BriefHelpCommand not yet implemented in cli_helpers.py") from docsible.cli import cli runner = CliRunner() @@ -75,9 +75,9 @@ def test_role_full_help_shows_more_options(self): def test_role_full_help_has_more_lines_than_brief(self): """Full help should produce more output lines than brief help.""" - try: - from docsible.utils.cli_helpers import BriefHelpCommand - except ImportError: + import docsible.utils.cli_helpers as cli_helpers + + if not hasattr(cli_helpers, "BriefHelpCommand"): pytest.skip("BriefHelpCommand not yet implemented in cli_helpers.py") from docsible.cli import cli runner = CliRunner() diff --git a/tests/phase3/test_intent_behavior.py b/tests/phase3/test_intent_behavior.py index 5fe7db5..7f778d1 100644 --- a/tests/phase3/test_intent_behavior.py +++ b/tests/phase3/test_intent_behavior.py @@ -50,7 +50,7 @@ def test_analyze_outputs_json_and_does_not_write_role_files(tmp_path): runner = CliRunner() with patch( - "docsible.commands.document_role.orchestrators.role_orchestrator.generate_all_recommendations", + "docsible.commands.document_role.role_analysis.generate_all_recommendations", return_value=[_recommendation()], ): result = runner.invoke( @@ -70,7 +70,7 @@ def test_analyze_json_keeps_suppression_notice_off_stdout(tmp_path): runner = CliRunner() with patch( - "docsible.commands.document_role.orchestrators.role_orchestrator.generate_all_recommendations", + "docsible.commands.document_role.role_analysis.generate_all_recommendations", return_value=[_recommendation()], ), patch( "docsible.suppression.engine.apply_suppressions", @@ -87,7 +87,7 @@ def test_analyze_json_outputs_empty_findings_when_all_are_suppressed(tmp_path): runner = CliRunner() with patch( - "docsible.commands.document_role.orchestrators.role_orchestrator.generate_all_recommendations", + "docsible.commands.document_role.role_analysis.generate_all_recommendations", return_value=[_recommendation()], ), patch( "docsible.suppression.engine.apply_suppressions", @@ -111,7 +111,7 @@ def test_validate_is_read_only_and_strict_uses_markdown_issues(tmp_path): ) with patch("docsible.validation.markdown_validator.MarkdownValidator.validate", return_value=[issue]), patch( - "docsible.commands.document_role.orchestrators.role_orchestrator.generate_all_recommendations", + "docsible.commands.document_role.role_analysis.generate_all_recommendations", return_value=[_recommendation()], ): result = runner.invoke( diff --git a/tests/phase3/test_wizard.py b/tests/phase3/test_wizard.py index 9460f5f..2d80ead 100644 --- a/tests/phase3/test_wizard.py +++ b/tests/phase3/test_wizard.py @@ -101,7 +101,7 @@ def test_no_force_existing_config_prompts_overwrite(self, tmp_path): def test_no_force_existing_config_aborts_on_no(self, tmp_path): runner = CliRunner() runner.invoke(wizard_init, ["--preset", "personal", "--path", str(tmp_path)]) - result = runner.invoke( + runner.invoke( wizard_init, ["--preset", "team", "--path", str(tmp_path)], input="n\n", From 9a51453a0721f43a5f49e4057de9b751c165523b Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 04:55:11 +0200 Subject: [PATCH 13/20] claims update --- CLAIMS.md | 147 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 126 insertions(+), 21 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index e0aaa3e..8c31833 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -1,6 +1,7 @@ # Docsible: Verified Project State -Snapshot: 2026-09-13 +Snapshot: 2026-09-13 (updated: Role Execution Graph completion, collection +support fixes, and role-analysis consolidation) ## Purpose @@ -53,29 +54,35 @@ The following commands were run for this snapshot: ```bash uv run pytest uv run ruff check . -uv run python -m build -npx --yes jscpd docsible +uv run mypy docsible ``` -- `uv run pytest` completed successfully. -- `uv run ruff check .` reported 47 findings, all in the test tree. Most are - import ordering or unused-import issues; the check is not currently clean. -- `uv run python -m build` successfully produced the source distribution and - wheel for `docsible-jier` version `0.9.0`. -- `npx --yes jscpd docsible` was run against the source tree only and found - duplicate code, including overlapping analyzer and command-orchestration - paths. +- `uv run pytest`: **1200 passed, 3 xpassed** (was 1156 passed at the + 2026-08-27 baseline; growth is new regression tests for the Role Execution + Graph, collection-support fixes, and the role-analysis consolidation below). +- `uv run ruff check .`: **all checks passed** (was 47 findings, all in the + test tree). The 5 findings remaining after the graph work (an undefined + `click` reference, unused defensive imports, an unused loop variable) have + since been fixed; the check is now clean. +- `uv run mypy docsible`: **no issues found in 236 source files**. +- `uv run python -m build` and `npx --yes jscpd docsible` were run for the + 2026-08-27 baseline only and have not been re-verified in this snapshot. ## Known Limitations - There is no GitHub Actions workflow in `.github/workflows`; tests, linting, builds, and CLI smoke checks are not yet run by repository CI. -- Ruff currently reports findings in the test tree. -- The analyzer migration is incomplete: role-analysis and role-documentation - orchestration retain duplicated implementation, including role-information - assembly in `complexity_analyzer` and `document_role`. +- `RoleInfoBuilder` (`docsible/commands/document_role/builders/role_info_builder.py`) + is still present as a deprecated, unused-in-production duplicate of + `RoleInfoLoader`. The role-analysis pipeline (complexity, execution graph, + recommendations) is now consolidated (see below); role-information + *loading* still has this one remaining duplicate implementation. - The deprecated `docsible role` command is still present alongside the newer intent-based command groups. +- Collections do not yet resolve cross-role boundaries: `include_role`/ + `import_role` targets pointing at a sibling role in the same collection are + recorded as `unresolved_external` rather than a real internal graph edge. + See Next Graph Milestones. ## Role Execution Graph @@ -101,10 +108,29 @@ interface is `build_role_execution_graph(role_info)`. Loop column when applicable. - Complexity reports retain their structural metrics and add graph metrics: statically reachable task files, dynamic and unknown boundaries, external - role references, loop tasks, notification edges, and orphan task files. + role references, loop tasks, notification edges, orphan task files, + collection dependencies, and conditional decision points. The full + serialized graph (`nodes`, `edges`) is attached to the complexity report and + exposed via `analyze role --output-format json`. - The graph uses standard-library dataclasses for a small serializable core. NetworkX is not a Docsible dependency; a future visualization adapter may convert the graph for layout algorithms. +- Templated include/import targets are classified, not just marked unknown: + a target matching `{{ role_path }}/tasks/` resolves statically; a + templated target with a literal filename prefix (e.g. + `install-{{ os_family }}.yml`) resolves to one or more `dynamic` candidate + edges against in-repo files with that prefix; fully unconstrained + expressions remain dynamic with no fabricated candidates. +- For roles classified `ENTERPRISE`, the README uses a bounded **grouped + execution overview** (directory-level groups, dynamic/unknown boundaries + summarized as counts, orphan files collapsed to one line) instead of a + detailed per-task-file diagram that would be unreadable at that size. The + detailed projection is used below a fixed budget (20 task files, 35 + relationship edges, fan-out 8); any signal exceeded switches projection. +- The rendered README leads with product value before task-file reference + material: Overview → Architecture Overview → Execution Graph Summary → + Execution Routes → Recommendations → Variable Reference → Task File + Reference → Handlers. ### Verified External Cases @@ -116,6 +142,21 @@ interface is `build_role_execution_graph(role_info)`. JSON parses, documentation generation succeeds, `main.yml` is Phase 1, the seven OS-specific branches retain their `when` conditions, and `vhosts.yml` is reached through its static import. +- `geerlingguy/ansible-role-mysql` @ `0a0ea6b728120b3ab3918332d9404bb65836834d`: + legacy `with_items`/`with_first_found` loops render in task tables; 9 static + include boundaries resolve; no false orphans. +- `geerlingguy/ansible-role-postgresql` @ `53abdf144de8231b2f2ce0652523eebc3eda7100`: + mixed `include_tasks`/`import_tasks` boundaries resolve; a 27-file `vars/` + directory renders without truncation. +- `nginx/ansible-role-nginx` (official) @ `157e0e97406f798bd6f50db37430a78c4269aa92`: + 244 tasks, 31 nested task files, classified `ENTERPRISE`. Verifies the + grouped execution overview and templated-target classification: 22 dynamic + candidate edges resolve against in-repo files, 0 unknown boundaries, 0 false + orphans (was 13 before dynamic-candidate resolution). +- `prometheus-community/ansible` @ `e2f46e17d33651c3c09042aaa9c8f29b87a9753f` + (the `prometheus.prometheus` collection, 26 roles): first real collection + exercised end-to-end; verifies collection support fixes and role-analysis + parity below. ### Completed Graph Milestones @@ -124,21 +165,85 @@ interface is `build_role_execution_graph(role_info)`. 3. Replace placeholder phases with source-backed Execution Routes. 4. Preserve static and dynamic cross-role boundaries in a JSON-serializable renderer contract. +5. Classify templated include/import targets as static, dynamic-candidate, or + genuinely unresolved, instead of leaving every templated target as an + orphan. +6. Add a bounded grouped-execution-overview projection for `ENTERPRISE` roles + so large role graphs stay readable instead of being suppressed entirely or + rendered as an unreadable wall of nodes. +7. Consolidate role complexity/execution-graph/recommendation analysis into + one shared implementation (`docsible/commands/document_role/role_analysis.py`: + `analyze_role()` + `render_analyzed_role()`), used identically by + `RoleOrchestrator` (standalone `document role`), `document_collection_roles()` + (`document role --collection`), and `scan/collection.py::_analyse_role()` + (`scan collection`). This fixed a real divergence: `scan collection` + previously computed recommendations without the complexity report and + silently missed the graph-aware findings the other two paths already had. +8. Give collection roles full parity with standalone roles: every role + documented via `document role --collection` now gets an identical + Architecture Overview, Execution Graph Summary, Execution Routes, and + Recommendations section (previously skipped entirely for collection + roles). +9. Add a collection-level **Complexity Overview** and **Role Index** to the + collection README: aggregate role counts by complexity category and total + task count, plus a per-role table (complexity badge, task count, + critical/warning counts, top finding) sorted by complexity descending, so + a reader sees which roles need attention before opening any of them. This + is deliberately an index of independent per-role facts, not a synthesized + single "collection complexity" score — a collection has no single + execution graph the way one role does. ### Next Graph Milestones 1. Publish a documented JSON graph contract after its node and edge fields are exercised by more external candidates. -2. Resolve locally available roles in sibling role directories and collections; - retain absent Galaxy/FQCN roles as explicit external-reference nodes. -3. Add graph projections for dynamic task/role includes, blocks, - rescue/always, and source-linked variable scopes without claiming static - certainty where Ansible defers resolution. +2. Resolve `include_role`/`import_role` targets that point at a sibling role + in the same collection into real internal graph edges, instead of + `unresolved_external`. Scoped narrowly to this one relationship (not a + full collection-wide dependency graph, which was assessed as low value: + readers almost always want one role's own dependencies, not a map of all + roles' relationships). +3. Add graph projections for blocks, rescue/always, and source-linked + variable scopes without claiming static certainty where Ansible defers + resolution. 4. Make `graph_visualisation` a renderer adapter over this contract, using NetworkX only for renderer-specific layout work. 5. Extend the pinned external corpus before treating the graph contract as release-stable. +## Collection Support + +`document role --collection` and `scan collection` were exercised end-to-end +for the first time against a real, non-trivial collection +(`prometheus-community/ansible`, 26 roles) and had several defects that a +smaller/synthetic test collection did not surface: + +- `document role --collection ... --dry-run` was not read-only: it wrote a + README backup before any short-circuit existed. Fixed with an explicit + dry-run check before any file is touched. +- `meta/argument_specs.yml` files using Ansible's `!unsafe` YAML tag failed to + load (`could not determine a constructor for the tag '!unsafe'`). Fixed + with a `DocsibleSafeLoader` that preserves the scalar value. +- The role README template referenced `sections/argument_specs.jinja2`, + which did not exist, crashing collection documentation for any role with + argument specs. The template was added. +- The collection-level README template referenced `role.belongs_to_collection` + in a macro where `role` was never in scope, raising + `jinja2.exceptions.UndefinedError` for any collection with a detectable + repository URL. Fixed: collection-level links always build the `roles/` + prefix, since collection templates are always in a collection context. +- Nearly every collection template (`overview.jinja2`, `roles_list.jinja2`, + `galaxy_info.jinja2`, `dependencies.jinja2`, `plugin_list.jinja2`, and the + `render_arguments_list` macro) was missing Jinja whitespace control, + producing a blank line after almost every list item, table row, and + argument-spec field — a generated collection README for a 26-role + collection was ~8000 lines, mostly blank. Fixed at the template level, and + `render_collection()` now applies the same blank-line normalization + (`MarkdownProcessor`) that `render_role()` already applied, capping any + residual run at 2 consecutive blank lines. +- `sections/overview.jinja2` printed a literal `\n` after every collection + author name (a template typo, not an escape sequence). Fixed. + ## Remaining Duplication Work The source-only duplication scan is below the original baseline, but remaining From 7dee44516f21a4a8710646d871c2058f82e76738 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 04:58:10 +0200 Subject: [PATCH 14/20] missing update of remaning work for duplicate work, collection boundary work within roles --- CLAIMS.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 8c31833..5f0ddcd 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -249,9 +249,12 @@ smaller/synthetic test collection did not surface: The source-only duplication scan is below the original baseline, but remaining duplication is prioritized by ownership and behavior rather than percentage. -1. Consolidate role-information assembly into one read-only loader used by - commands and analyzers. This is the highest priority because duplicate - loaders previously produced divergent behavior. +1. **Partially resolved.** Role complexity/execution-graph/recommendation + *analysis* is now consolidated in `role_analysis.py` (see Role Execution + Graph, milestone 7) and used identically by `document role`, + `document role --collection`, and `scan collection`. Role-information + *loading* still has one remaining duplicate: `RoleInfoBuilder` alongside + `RoleInfoLoader` (see Known Limitations). 2. Consider a private helper for repeated integration-provider task traversal after the role-loader migration is complete. 3. Review overlapping renderer model fields only when a concrete rendering From 0ae3bbc17ba43d9794cd8cb16491d01ce58ec865 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 17:36:06 +0200 Subject: [PATCH 15/20] =?UTF-8?q?fix:=20consistent=20collection=20role=20d?= =?UTF-8?q?iscovery=20and=20skip=20empty=20submodule=20dirs=20-=20document?= =?UTF-8?q?=5Fcollection=5Froles()=20and=20my=20dry-run=20now=20iterate=20?= =?UTF-8?q?ProjectStructure.find=5Froles()=20(the=20same=20filter=20scan?= =?UTF-8?q?=20uses)=20instead=20of=20os.listdir/iterdir=20of=20roles/*.=20?= =?UTF-8?q?-=20New=20=5Frole=5Fless=5Fdirs=20+=20=5Fwarn=5Frole=5Fless=5Fd?= =?UTF-8?q?irs:=20role-less=20dirs=20(uninitialized=20submodules)=20are=20?= =?UTF-8?q?skipped,=20warned,=20and=20excluded=20from=20the=20dry-run=20co?= =?UTF-8?q?unt=20and=20the=20collection=20Role=20Index=20=E2=80=94=20and?= =?UTF-8?q?=20never=20written=20into.=20feat:=20capture=20loop=5Fcontrol?= =?UTF-8?q?=20and=20render=20custom=20loop=20variables=20-=20Shared=20extr?= =?UTF-8?q?act=5Floop=5Fcontrol()=20in=20special=5Ftasks=5Fkeys=20(?= =?UTF-8?q?=E2=86=92=20loop=20stays=20the=20keyword;=20loop=5Fvar/index=5F?= =?UTF-8?q?var/label=20captured;=20pause=20intentionally=20excluded).=20-?= =?UTF-8?q?=20Graph=20task-node=20metadata=20now=20carries=20loop=5Fcontro?= =?UTF-8?q?l;=20serialized=20into=20to=5Fdict().=20-=20README=20Loop=20col?= =?UTF-8?q?umn=20renders=20loop=20(as:=20)=20in=20both=20standard=20+?= =?UTF-8?q?=20hybrid=20templates.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAIMS.md | 39 +++++++++++++- docsible/commands/document_collection.py | 54 ++++++++++++++----- docsible/graphs/role_execution.py | 4 ++ docsible/templates/role/sections/tasks.jinja2 | 4 +- .../role/sections/tasks_hybrid.jinja2 | 4 +- docsible/utils/special_tasks_keys.py | 19 +++++++ .../document_role/test_role_analysis.py | 32 +++++++++++ tests/commands/test_document_collection.py | 38 +++++++++++++ tests/graphs/test_role_execution.py | 42 ++++++++++++++- tests/utils/test_special_tasks_keys.py | 24 +++++++++ 10 files changed, 243 insertions(+), 17 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 5f0ddcd..24de8d3 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -104,8 +104,12 @@ interface is `build_role_execution_graph(role_info)`. - Component architecture diagrams derive variable and handler edges from graph facts, replacing the old first-file and last-file proxy edges. - Task nodes preserve modern and legacy loop syntax (`loop`, `with_items`, - `with_first_found`, and other `with_*` forms); README task tables render a - Loop column when applicable. + `with_first_found`, and other `with_*` forms), and structured + `loop_control` — `loop_var`, `index_var`, `label` — is captured into the + task-node metadata and the serialized graph, and surfaced in the README + Loop column as `loop (as: )`. This makes custom loop variables visible + (they are loop-local, not role variables) for both humans and external + renderers. - Complexity reports retain their structural metrics and add graph metrics: statically reachable task files, dynamic and unknown boundaries, external role references, loop tasks, notification edges, orphan task files, @@ -157,6 +161,14 @@ interface is `build_role_execution_graph(role_info)`. (the `prometheus.prometheus` collection, 26 roles): first real collection exercised end-to-end; verifies collection support fixes and role-analysis parity below. +- `dev-sec/ansible-collection-hardening` @ `3102eddbd116c5f8c1581aca543d372dbc326764` + (the `devsec.hardening` collection; loop/condition-heavy; 4 real roles + + 2 uninitialized submodule dirs under `roles/`): verifies the loop_control + capture (2 `loop_var` usages now render as `loop (as: …)`), the collection + discovery parity (scan and `--collection` both report 4; the 2 empty + submodule dirs are skipped/warned and excluded from the index), and the + ENTERPRISE grouped diagram still reachable via explicit `--graph` + (`os_hardening`, 125 tasks / 23 files). ### Completed Graph Milestones @@ -192,6 +204,20 @@ interface is `build_role_execution_graph(role_info)`. is deliberately an index of independent per-role facts, not a synthesized single "collection complexity" score — a collection has no single execution graph the way one role does. +10. Capture `loop_control` (`loop_var`/`index_var`/`label`) into task-node + metadata and the serialized graph, and surface it in the README Loop + column as `loop (as: )`. (Candidate 7 finding: source `loop_var` + appeared in 0 of 2 generated READMEs; now 2 of 2 render, with the graph + carrying the metadata.) +11. Make collection role discovery consistent: `document role --collection` + now iterates `ProjectStructure.find_roles()` (the same filter `scan + collection` uses), so the two can no longer disagree on what is a role. + Role-less `roles/*` directories (e.g. uninitialized git submodules with + no `tasks`/`defaults`/`vars`/`meta`) are skipped, warned about, excluded + from the dry-run count and the collection Role Index, and never written + into. (Candidate 7 finding: `scan` found 4, `--collection` documented 6, + and 2 stub READMEs were silently written into empty submodule dirs + invisible to the parent `git status`; now 4 everywhere with warnings.) ### Next Graph Milestones @@ -243,6 +269,15 @@ smaller/synthetic test collection did not surface: residual run at 2 consecutive blank lines. - `sections/overview.jinja2` printed a literal `\n` after every collection author name (a template typo, not an escape sequence). Fixed. +- The per-role loop used a raw `os.listdir` of `roles/`, so it disagreed with + `scan`'s `find_roles` filter and treated uninitialized git submodules + (empty `roles/*` dirs) as zero-content roles: it miscounted in `--dry-run`, + wrote stub `README.md`/`.docsible` into submodule paths invisible to the + parent `git status`, and inflated the collection Role Index. Fixed: the + collection path now iterates `ProjectStructure.find_roles()` and skips + + warns on role-less dirs. (Found via + `dev-sec/ansible-collection-hardening`, which has 4 real roles + 2 empty + submodule dirs under `roles/`.) ## Remaining Duplication Work diff --git a/docsible/commands/document_collection.py b/docsible/commands/document_collection.py index 768d44a..2615269 100644 --- a/docsible/commands/document_collection.py +++ b/docsible/commands/document_collection.py @@ -1,7 +1,6 @@ """Command for documenting Ansible collections.""" import logging -import os from pathlib import Path import click @@ -17,6 +16,33 @@ logger = logging.getLogger(__name__) +def _role_less_dirs(roles_dir: Path, valid_roles: list[Path]) -> list[str]: + """Names of immediate ``roles/*`` subdirectories that are not valid roles. + + A directory counts as a role only if ``find_roles`` accepts it (has + tasks/defaults/vars/meta content). Empty dirs — typically uninitialized + git submodules — land here and must be reported, not documented as + zero-content roles. + """ + if not roles_dir.is_dir(): + return [] + valid = {path.resolve() for path in valid_roles} + return sorted( + entry.name + for entry in roles_dir.iterdir() + if entry.is_dir() and entry.resolve() not in valid + ) + + +def _warn_role_less_dirs(names: list[str]) -> None: + for name in names: + logger.warning( + "Skipping roles/%s: no role content found " + "(no tasks/defaults/vars/meta; possibly an uninitialized submodule).", + name, + ) + + def document_collection_roles( collection_path: str, playbook: str | None, @@ -105,12 +131,14 @@ def document_collection_roles( return if dry_run: - role_count = sum( - 1 - for marker in collection_markers - for role_path in ProjectStructure(str(marker.parent)).get_roles_dir().iterdir() - if role_path.is_dir() - ) + role_count = 0 + skipped: list[str] = [] + for marker in collection_markers: + structure = ProjectStructure(str(marker.parent)) + valid_roles = structure.find_roles() + role_count += len(valid_roles) + skipped.extend(_role_less_dirs(structure.get_roles_dir(), valid_roles)) + _warn_role_less_dirs(skipped) click.echo(f"Dry-run: would document {role_count} role(s) in {collection_path}") return @@ -138,12 +166,14 @@ def document_collection_roles( roles_dir = collection_structure.get_roles_dir() roles_info = [] + # Use the same role discovery as `scan collection` (find_roles filters + # on real role content), so the two commands can never disagree about + # which directories are roles, and role-less dirs are never rendered. + valid_roles = collection_structure.find_roles() + _warn_role_less_dirs(_role_less_dirs(roles_dir, valid_roles)) if roles_dir.exists() and roles_dir.is_dir(): - for role_name in os.listdir(str(roles_dir)): - role_path = roles_dir / role_name - - if not role_path.is_dir(): - continue + for role_path in sorted(valid_roles, key=lambda path: path.name): + role_name = role_path.name # Load playbook content if specified playbook_content = None diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index 78cbf87..0e37387 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -10,6 +10,8 @@ from pathlib import PurePosixPath from typing import Any +from docsible.utils.special_tasks_keys import extract_loop_control + class NodeKind(str, Enum): ROLE = "role" @@ -238,6 +240,8 @@ def _add_tasks(graph: RoleExecutionGraph, role_name: str, task_file: dict[str, A metadata["condition"] = condition if loop := _loop(task): metadata["loop"] = loop + if loop_control := extract_loop_control(task): + metadata["loop_control"] = loop_control graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, metadata)) graph.add_edge(GraphEdge(EdgeKind.CONTAINS, file_ids[file_name], task_id, ResolutionStatus.STATIC, source)) _add_variable_edges(graph, task_id, task, variables, source) diff --git a/docsible/templates/role/sections/tasks.jinja2 b/docsible/templates/role/sections/tasks.jinja2 index 8fd828b..a18b3dc 100644 --- a/docsible/templates/role/sections/tasks.jinja2 +++ b/docsible/templates/role/sections/tasks.jinja2 @@ -16,7 +16,9 @@ |-----------|--------|----------------|{% if ns.has_loops %}------|{% endif %}{% if ns.has_tags %}------|{% endif %}{% if ns.has_comments %}-------------|{% endif %} {% for task in task_file.tasks -%} {%- set link = links.render_repo_link(role.repository, role.name, 'tasks/' ~ task_file.file, task_file.lines[task.name], role.repository_type, role.repository_branch, role.belongs_to_collection) -%} -| {% if task_file.lines and task_file.lines[task.name] %}[{{ task.name | escape_table_cell }}]({{ link }}){% else %}{{ task.name | escape_table_cell }}{% endif %} | {{ task.module | escape_table_cell }} | {{ 'True' if task.when else 'False' }} |{% if ns.has_loops %} {{ task.loop | default('') | escape_table_cell }} |{% endif %}{% if ns.has_tags %} {{ task_file.mermaid | selectattr('name', 'equalto', task.name) | map(attribute='tags') | safe_join(',') }} |{% endif %}{% if ns.has_comments %} {{ task_file.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} +{%- set loop_text = task.loop | default('') -%} +{%- if task.loop_control is defined and task.loop_control.loop_var is defined -%}{%- set loop_text = loop_text ~ ' (as: ' ~ task.loop_control.loop_var ~ ')' -%}{%- endif -%} +| {% if task_file.lines and task_file.lines[task.name] %}[{{ task.name | escape_table_cell }}]({{ link }}){% else %}{{ task.name | escape_table_cell }}{% endif %} | {{ task.module | escape_table_cell }} | {{ 'True' if task.when else 'False' }} |{% if ns.has_loops %} {{ loop_text | escape_table_cell }} |{% endif %}{% if ns.has_tags %} {{ task_file.mermaid | selectattr('name', 'equalto', task.name) | map(attribute='tags') | safe_join(',') }} |{% endif %}{% if ns.has_comments %} {{ task_file.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} {% endfor %} {# Mermaid diagram section with pagination support #} diff --git a/docsible/templates/role/sections/tasks_hybrid.jinja2 b/docsible/templates/role/sections/tasks_hybrid.jinja2 index 2af80c2..75beef6 100644 --- a/docsible/templates/role/sections/tasks_hybrid.jinja2 +++ b/docsible/templates/role/sections/tasks_hybrid.jinja2 @@ -7,7 +7,9 @@ | Task Name | Module |{% if ns.has_loops %} Loop |{% endif %}{% if ns.has_comments %} Description |{% endif %} |-----------|--------|{% if ns.has_loops %}------|{% endif %}{% if ns.has_comments %}-------------|{% endif %} {%- for task in task_info.tasks %} -| {{ task.name | escape_table_cell if task.name else '*unnamed*' }} | {{ task.module | escape_table_cell }} |{% if ns.has_loops %} {{ task.loop | default('') | escape_table_cell }} |{% endif %}{% if ns.has_comments %} {{ task_info.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} +{%- set loop_text = task.loop | default('') -%} +{%- if task.loop_control is defined and task.loop_control.loop_var is defined -%}{%- set loop_text = loop_text ~ ' (as: ' ~ task.loop_control.loop_var ~ ')' -%}{%- endif -%} +| {{ task.name | escape_table_cell if task.name else '*unnamed*' }} | {{ task.module | escape_table_cell }} |{% if ns.has_loops %} {{ loop_text | escape_table_cell }} |{% endif %}{% if ns.has_comments %} {{ task_info.comments | selectattr('task_name', 'equalto', task.name) | map(attribute='task_comments') | safe_join(' ') }} |{% endif %} {%- endfor %} {% endif %} diff --git a/docsible/utils/special_tasks_keys.py b/docsible/utils/special_tasks_keys.py index 513b135..3456b61 100644 --- a/docsible/utils/special_tasks_keys.py +++ b/docsible/utils/special_tasks_keys.py @@ -214,7 +214,26 @@ def process_special_task_keys( ) if loop_key is not None: processed_task["loop"] = loop_key + if loop_control := extract_loop_control(task): + processed_task["loop_control"] = loop_control if include_target is not None: processed_task["include_target"] = include_target tasks.append(processed_task) return tasks + + +def extract_loop_control(task: dict[str, Any]) -> dict[str, Any]: + """Extract change-relevant ``loop_control`` fields from a raw task. + + A custom ``loop_var``/``index_var``/``label`` is loop-local variable + binding, not a role variable, so it must be captured for both README + rendering and execution-graph variable scoping. + """ + control = task.get("loop_control") + if not isinstance(control, dict): + return {} + return { + key: control[key] + for key in ("loop_var", "index_var", "label") + if key in control + } diff --git a/tests/commands/document_role/test_role_analysis.py b/tests/commands/document_role/test_role_analysis.py index 782b6d9..268067a 100644 --- a/tests/commands/document_role/test_role_analysis.py +++ b/tests/commands/document_role/test_role_analysis.py @@ -136,3 +136,35 @@ def test_no_excessive_blank_lines(self, large_role): else: blank_run = 0 assert max_blank_run <= 2 + + +class TestLoopControlRendering: + def test_readme_loop_column_shows_custom_loop_var(self, tmp_path): + role = tmp_path / "loop_role" + (role / "tasks").mkdir(parents=True) + (role / "tasks" / "main.yml").write_text( + "---\n" + "- name: Loop with custom variable\n" + " ansible.builtin.debug:\n" + " msg: '{{ entry }}'\n" + " loop:\n" + " - a\n" + " - b\n" + " loop_control:\n" + " loop_var: entry\n" + ) + (role / "handlers").mkdir() + (role / "defaults").mkdir() + (role / "defaults" / "main.yml").write_text("---\nx: 1\n") + + role_info = RoleInfoLoader().load(role) + analysis = analyze_role(role_info, role) + render_analyzed_role( + role_info=role_info, + role_path=role, + analysis=analysis, + output_path=role / "README.md", + ) + + readme = (role / "README.md").read_text() + assert "loop (as: entry)" in readme diff --git a/tests/commands/test_document_collection.py b/tests/commands/test_document_collection.py index a85adbb..afe9336 100644 --- a/tests/commands/test_document_collection.py +++ b/tests/commands/test_document_collection.py @@ -286,3 +286,41 @@ def test_role_index_sorts_most_complex_role_first(self, tmp_path): assert readme.index("db_role", index_start) < readme.index( "proxy_role", index_start ) + + +class TestRoleLessDirHandling: + """Empty/non-role directories under roles/ (e.g. uninitialized git + submodules) must be skipped and reported, never documented as zero-task + roles, and never counted in the collection index. This is the candidate-7 + finding fix and keeps `document --collection` in parity with `scan`. + """ + + def _with_stub_dir(self, tmp_path): + collection = _copy_collection(MULTI_ROLE_COLLECTION, tmp_path / "collection") + (collection / "roles" / "uninitialized_submodule_stub").mkdir() + return collection + + def test_role_less_dir_not_documented(self, tmp_path): + collection = self._with_stub_dir(tmp_path) + + _document(collection, dry_run=False) + + stub = collection / "roles" / "uninitialized_submodule_stub" + assert not (stub / "README.md").exists() + assert not (stub / ".docsible").exists() + + def test_role_less_dir_excluded_from_collection_index(self, tmp_path): + collection = self._with_stub_dir(tmp_path) + + _document(collection, dry_run=False) + + readme = (collection / "README.md").read_text() + assert "**3 roles**" in readme # cache/db/proxy; the stub is excluded + assert "uninitialized_submodule_stub" not in readme + + def test_dry_run_counts_only_valid_roles(self, tmp_path, capsys): + collection = self._with_stub_dir(tmp_path) + + _document(collection, dry_run=True) + + assert "would document 3 role(s)" in capsys.readouterr().out diff --git a/tests/graphs/test_role_execution.py b/tests/graphs/test_role_execution.py index c766884..de14bb7 100644 --- a/tests/graphs/test_role_execution.py +++ b/tests/graphs/test_role_execution.py @@ -2,7 +2,12 @@ import json -from docsible.graphs import EdgeKind, ResolutionStatus, build_role_execution_graph +from docsible.graphs import ( + EdgeKind, + NodeKind, + ResolutionStatus, + build_role_execution_graph, +) def test_builds_static_dynamic_and_notify_relationships(): @@ -79,3 +84,38 @@ def test_preserves_external_role_boundaries_in_renderer_contract(): assert role_edges[1].resolution is ResolutionStatus.DYNAMIC assert role_edges[1].target_id is None json.dumps(graph.to_dict()) + + +def test_loop_control_recorded_in_task_metadata(): + graph = build_role_execution_graph( + { + "name": "web", + "defaults": [], + "vars": [], + "handlers": [], + "tasks": [ + { + "file": "main.yml", + "tasks": [{}], + "line_ranges": [(1, 3)], + "mermaid": [ + { + "name": "Loop custom var", + "ansible.builtin.debug": {}, + "loop": ["a", "b"], + "loop_control": {"loop_var": "entry", "index_var": "i"}, + } + ], + } + ], + } + ) + + loop_nodes = [ + node + for node in graph.nodes.values() + if node.kind is NodeKind.TASK and "loop_control" in node.metadata + ] + assert loop_nodes, "loop_control must be recorded on the task node" + assert loop_nodes[0].metadata["loop"] == "loop" + assert loop_nodes[0].metadata["loop_control"] == {"loop_var": "entry", "index_var": "i"} diff --git a/tests/utils/test_special_tasks_keys.py b/tests/utils/test_special_tasks_keys.py index d20d597..34b218c 100644 --- a/tests/utils/test_special_tasks_keys.py +++ b/tests/utils/test_special_tasks_keys.py @@ -92,3 +92,27 @@ def test_block_task_shape_unchanged(): assert result[1]["module"] == "debug" # An empty rescue list contributes no rescue entry assert len(result) == 2 + + +def test_loop_control_captured_with_loop(): + result = process_special_task_keys({ + "name": "Iterate", + "ansible.builtin.debug": {}, + "loop": ["a", "b"], + "loop_control": {"loop_var": "entry", "index_var": "i", "label": "{{ entry }}", "pause": 5}, + }) + assert result[0]["loop"] == "loop" + # loop_var/index_var/label captured; pause (a control knob, not a name) is not + assert result[0]["loop_control"] == {"loop_var": "entry", "index_var": "i", "label": "{{ entry }}"} + + +def test_loop_control_absent_for_plain_loop(): + result = process_special_task_keys({"debug": {"msg": "x"}, "loop": ["a"]}) + assert result[0]["loop"] == "loop" + assert "loop_control" not in result[0] + + +def test_loop_control_ignored_without_a_loop(): + result = process_special_task_keys({"name": "x", "debug": {}, "loop_control": {"loop_var": "v"}}) + assert "loop" not in result[0] + assert "loop_control" not in result[0] From 9d05a335239645918c8668053590a6141b86fb4e Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 18:12:06 +0200 Subject: [PATCH 16/20] help user when using --graph to expect per task file on onlyh simple/medium code base and not on enterprise graded project. --- CLAIMS.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CLAIMS.md b/CLAIMS.md index 24de8d3..84e421a 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -236,6 +236,15 @@ interface is `build_role_execution_graph(role_info)`. NetworkX only for renderer-specific layout work. 5. Extend the pinned external corpus before treating the graph contract as release-stable. +6. (Finding D, deferred) Surface diagram tiering for `--graph`: COMPLEX and + ENTERPRISE roles currently render only the file-level architecture + diagram; per-task-file flow diagrams are shown only for SIMPLE/MEDIUM. + This is by design (a task-level graph is unreadable at that size), not a + defect, so no inline disclaimer is added yet. The genuine remedy is to + expose task-level flow through the interactive `graph_visualisation` + adapter (item 4), where the "too large for Mermaid" content belongs; the + README note, if ever needed, should be written once that adapter exists so + it does not churn. ## Collection Support From 11ae83713fc3f7aee95d022101d6e761737d8d65 Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 18:49:18 +0200 Subject: [PATCH 17/20] =?UTF-8?q?fix:=20bound=20grouped=20execution=20over?= =?UTF-8?q?view=20for=20flat=20task=20directories=20docsible/diagrams/type?= =?UTF-8?q?s/architecture.py=20=E2=80=94=20grouped=20overview=20now=20caps?= =?UTF-8?q?=20visible=20groups=20(=5FMAX=5FGROUP=5FNODES=20=3D=2010):=20ke?= =?UTF-8?q?ep=20entry=20point=20+=20largest=20groups=20by=20task=20count,?= =?UTF-8?q?=20fold=20the=20tail=20into=20one=20other=20(N=20task=20files)?= =?UTF-8?q?=20node=20(hub=20fan-out=20collapses=20via=20existing=20edge=20?= =?UTF-8?q?dedup).=20Plus=20label=20pluralization=20fix=20(1=20task=20file?= =?UTF-8?q?=20vs=20N=20task=20files).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAIMS.md | 15 +++++ docsible/diagrams/types/architecture.py | 39 +++++++++++-- tests/diagrams/test_architecture_diagram.py | 63 +++++++++++++++++++++ 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 84e421a..e91bd93 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -218,6 +218,21 @@ interface is `build_role_execution_graph(role_info)`. into. (Candidate 7 finding: `scan` found 4, `--collection` documented 6, and 2 stub READMEs were silently written into empty submodule dirs invisible to the parent `git status`; now 4 everywhere with warnings.) +12. Bound the grouped-execution overview for flat task directories. Grouping + keyed on the first path segment, so a nested layout already compresses + (official nginx: 31 files → 9 directory nodes, 50 mermaid lines) but a + flat layout degenerated to one node per file (os_hardening: 23 files → 23 + nodes, 100 lines — the densest diagram in the corpus). Added a hard node + cap (`_MAX_GROUP_NODES = 10`): when grouping yields more groups than the + cap, keep the entry point and the largest groups by task count and fold + the remainder into one `other (N task files)` node; hub fan-out into the + bucket collapses via the existing edge dedup. Verified: os_hardening + 23 → 11 nodes (100 → 52 lines) with an `other (13 task files)` bucket; + nested layouts at or under the cap render unchanged (official nginx stays + at 9 directory nodes, no bucket). Also fixed label pluralization (`1 task + file` vs `N task files`). This subsumes finding B (nested roles already + give the bounded multi-level view); finding D stays deferred (see Next + Graph Milestones). ### Next Graph Milestones diff --git a/docsible/diagrams/types/architecture.py b/docsible/diagrams/types/architecture.py index 1e6de70..9b5c343 100644 --- a/docsible/diagrams/types/architecture.py +++ b/docsible/diagrams/types/architecture.py @@ -13,6 +13,11 @@ _DETAILED_FILE_BUDGET = 20 _DETAILED_EDGE_BUDGET = 35 _DETAILED_FANOUT_BUDGET = 8 +# Hard cap on visible group nodes in the grouped overview, so a flat tasks/ +# directory (where each file is its own pseudo-group) cannot produce a wall of +# near-identical single-file nodes. Kept above real nested layouts (e.g. the +# official nginx role's 9 directories) so those render unchanged. +_MAX_GROUP_NODES = 10 def generate_component_architecture( @@ -245,20 +250,46 @@ def _should_group_execution_graph(execution_graph: Any, task_files: list[dict[st def _generate_grouped_architecture(role_info: dict[str, Any], execution_graph: Any) -> str: """Render a bounded directory-level overview from graph facts.""" task_nodes = [node for node in execution_graph.nodes.values() if node.kind.value == "task_file"] + file_task_count = { + node.metadata["file"]: node.metadata.get("task_count", 0) for node in task_nodes + } + groups: dict[str, list[str]] = {} - for node in task_nodes: - file_name = node.metadata["file"] + for file_name in file_task_count: group = "entry point" if file_name == "main.yml" else file_name.split("/", 1)[0] groups.setdefault(group, []).append(file_name) + # Bound the overview: keep the entry point and the largest groups by task + # count, and fold everything else into a single ``other`` bucket. This only + # engages when grouping fails to compress (flat directories), leaving nested + # layouts like the official nginx role untouched. + if len(groups) > _MAX_GROUP_NODES: + ranked = sorted( + (group for group in groups if group != "entry point"), + key=lambda group: sum(file_task_count[file] for file in groups[group]), + reverse=True, + ) + keep = {"entry point", *ranked[: _MAX_GROUP_NODES - 1]} + other_files = [ + file_name for group, files in groups.items() if group not in keep for file_name in files + ] + groups = {group: files for group, files in groups.items() if group in keep} + if other_files: + groups["other"] = sorted(other_files) + def node_id(group: str) -> str: return "group_" + re.sub(r"[^A-Za-z0-9_]", "_", group) + def group_label(group: str, files: list[str]) -> str: + if group == "entry point": + return "main.yml" + noun = "task file" if len(files) == 1 else "task files" + return f"{group}
{len(files)} {noun}" + file_group = {file_name: group for group, files in groups.items() for file_name in files} lines = ["graph TB", ' overview["Grouped execution overview"]'] for group, files in sorted(groups.items()): - label = "main.yml" if group == "entry point" else f"{group}
{len(files)} task files" - lines.append(f' {node_id(group)}["{label}"]') + lines.append(f' {node_id(group)}["{group_label(group, files)}"]') lines.append(f" overview --> {node_id(group)}") task_to_file = { diff --git a/tests/diagrams/test_architecture_diagram.py b/tests/diagrams/test_architecture_diagram.py index 53912d9..7a35074 100644 --- a/tests/diagrams/test_architecture_diagram.py +++ b/tests/diagrams/test_architecture_diagram.py @@ -480,3 +480,66 @@ def test_diagram_has_valid_mermaid_syntax(self): assert "-->" in diagram or "-.notify.->" in diagram assert "classDef" in diagram assert "class" in diagram + + +def _role_info_with_files(files): + return { + "name": "r", + "defaults": [], + "vars": [], + "handlers": [], + "tasks": [{"file": name, "tasks": [{}] * n} for name, n in files], + } + + +def _group_node_count(mermaid): + return sum( + 1 + for line in mermaid.splitlines() + if line.lstrip().startswith("group_") and "[" in line + ) + + +class TestGroupedOverviewBounding: + """Regression tests for the flat-layout grouping failure found while + re-running candidate 7 (os_hardening: 23 flat task files became 23 + 'groups' -> an unreadable 100-line diagram). Nested layouts (candidate 5, + 9 directory groups) must stay untouched. + """ + + def test_flat_layout_is_bounded_with_other_bucket(self): + from docsible.diagrams.types.architecture import _MAX_GROUP_NODES + from docsible.graphs import build_role_execution_graph + + files = [("main.yml", 2)] + [(f"sec_{i}.yml", 1) for i in range(22)] + role_info = _role_info_with_files(files) + graph = build_role_execution_graph(role_info) + + mermaid = generate_component_architecture( + role_info, None, execution_graph=graph + ) + + assert "Grouped execution overview" in mermaid + assert "group_other[" in mermaid + assert "13 task files" in mermaid # 23 - entry - 9 largest folded into other + assert _group_node_count(mermaid) <= _MAX_GROUP_NODES + 1 + assert '1 task file"' in mermaid # singular for one-file groups + assert "1 task files" not in mermaid # no plural on a single file + + def test_nested_layout_within_cap_is_not_folded(self): + from docsible.graphs import build_role_execution_graph + + files = [("main.yml", 1)] + [ + (f"dir_{d}/x{i}.yml", 1) for d in range(8) for i in range(3) + ] # 25 files (>20 -> grouped) but 8 directories + entry = 9 groups + role_info = _role_info_with_files(files) + graph = build_role_execution_graph(role_info) + + mermaid = generate_component_architecture( + role_info, None, execution_graph=graph + ) + + assert "Grouped execution overview" in mermaid + assert "group_other[" not in mermaid # 9 groups <= cap -> no folding + assert _group_node_count(mermaid) == 9 # entry + 8 directories + assert "3 task files" in mermaid # each directory keeps 3 files From 6ed29adbd8e001703201a3c57f41030fc46d26fb Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 19:39:45 +0200 Subject: [PATCH 18/20] perf: scan task variables once instead of per-variable regex then refactor: build the RoleExecutionGraph once per command and thread it. --- CLAIMS.md | 17 ++++++++++++ .../analyzers/role_analyzer.py | 7 ++++- docsible/commands/document_collection.py | 1 + .../orchestrators/role_orchestrator.py | 20 ++++++++++---- .../commands/document_role/role_analysis.py | 18 ++++++++++--- docsible/graphs/role_execution.py | 9 +++++-- tests/graphs/test_role_execution.py | 26 +++++++++++++++++++ 7 files changed, 87 insertions(+), 11 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index e91bd93..88421d0 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -260,6 +260,23 @@ interface is `build_role_execution_graph(role_info)`. adapter (item 4), where the "too large for Mermaid" content belongs; the README note, if ever needed, should be written once that adapter exists so it does not churn. +7. (Finding, deferred — precision, not perf) `uses_variable` edges over-match + because `_add_variable_edges` marks a variable "used" when its name appears + as *any* identifier token in `str(task)` — the whole serialized task + (module name, arg values, `when`/`register`, task-name prose, handler + names). The (a) perf fix kept this behavior identical (single tokenizer pass + instead of per-variable regex) but did not narrow it, so the edges can be + spurious: at CIS scale 1285 of 2510 edges are `uses_variable`, a plausible + share not real Jinja references. This weakens the JSON graph contract, the + "which variables does this task read" change-impact answer, and any dense + interactive variable layer. Planned fix (a deliberate semantics change, NOT + to be slipped into an optimization): restrict to genuine references — + `{{ name }}`/`{{ name.attr }}` interpolations and templated arg/`when` + values (reuse `dependency_matrix.extract_variable_references`) — exclude + non-reference keys, and treat `loop_control.loop_var`/`index_var` names as + loop-local so they are not linked to same-named role variables. Because it + intentionally drops noisy edges (fewer, more-accurate), it needs its own + tests and review. ## Collection Support diff --git a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py index b8ead91..95bb8da 100644 --- a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py +++ b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py @@ -106,6 +106,7 @@ def analyze_role_complexity( role_info: dict[str, Any], include_patterns: bool = False, min_confidence: float = 0.7, + execution_graph: Any | None = None, ) -> ComplexityReport: """Analyze role complexity and generate comprehensive report. @@ -113,6 +114,9 @@ def analyze_role_complexity( role_info: Role information dictionary from build_role_info() include_patterns: Whether to include pattern analysis (requires --simplification-report flag) min_confidence: Minimum confidence threshold for pattern detection (0.0-1.0) + execution_graph: Prebuilt RoleExecutionGraph to reuse instead of + rebuilding one (build is cheap now, but callers that already hold + one — e.g. analyze_role — pass it to avoid duplicate work) Returns: ComplexityReport with metrics, category, recommendations, and optional pattern analysis @@ -206,7 +210,8 @@ def analyze_role_complexity( # Create metrics from docsible.graphs import EdgeKind, NodeKind, ResolutionStatus, build_role_execution_graph - execution_graph = build_role_execution_graph(role_info) + if execution_graph is None: + execution_graph = build_role_execution_graph(role_info) phases = execution_graph.execution_phases() graph_metrics = { "static_reachable_task_files": sum( diff --git a/docsible/commands/document_collection.py b/docsible/commands/document_collection.py index 2615269..eb8d25e 100644 --- a/docsible/commands/document_collection.py +++ b/docsible/commands/document_collection.py @@ -235,6 +235,7 @@ def document_collection_roles( append=append, backup=not no_backup, playbook_content=playbook_content, + execution_graph=analysis.execution_graph, ) warning_count = sum( diff --git a/docsible/commands/document_role/orchestrators/role_orchestrator.py b/docsible/commands/document_role/orchestrators/role_orchestrator.py index 6b9a6dc..f37f541 100644 --- a/docsible/commands/document_role/orchestrators/role_orchestrator.py +++ b/docsible/commands/document_role/orchestrators/role_orchestrator.py @@ -6,6 +6,7 @@ import logging from pathlib import Path +from typing import Any import click @@ -74,8 +75,10 @@ def execute(self) -> None: self._display_analysis_and_exit(analysis_report, role_info) return - # Step 6: Generate diagrams - diagrams = self._generate_diagrams(role_info, analysis_report, playbook_content) + # Step 6: Generate diagrams (reuse the shared execution graph) + diagrams = self._generate_diagrams( + role_info, analysis_report, playbook_content, analysis.execution_graph + ) # Step 7: Generate dependency matrix dependency_data = self._generate_dependencies(role_info, analysis_report) @@ -259,7 +262,11 @@ def _display_analysis_and_exit(self, analysis_report, role_info: dict) -> None: handle_analyze_only_mode(role_info, role_info.get("name", "unknown")) def _generate_diagrams( - self, role_info: dict, analysis_report, playbook_content: str | None + self, + role_info: dict, + analysis_report, + playbook_content: str | None, + execution_graph: Any | None = None, ) -> dict: """Generate all Mermaid diagrams. @@ -267,6 +274,7 @@ def _generate_diagrams( role_info: Role information dictionary analysis_report: Complexity analysis report playbook_content: Optional playbook content + execution_graph: Prebuilt graph to reuse (built here only if absent) Returns: Dictionary of generated diagrams @@ -275,9 +283,11 @@ def _generate_diagrams( generate_integration_and_architecture_diagrams, generate_mermaid_diagrams, ) - from docsible.graphs import build_role_execution_graph - execution_graph = build_role_execution_graph(role_info) + if execution_graph is None: + from docsible.graphs import build_role_execution_graph + + execution_graph = build_role_execution_graph(role_info) # Generate task diagrams diagrams = generate_mermaid_diagrams( diff --git a/docsible/commands/document_role/role_analysis.py b/docsible/commands/document_role/role_analysis.py index abb8f0c..43b1602 100644 --- a/docsible/commands/document_role/role_analysis.py +++ b/docsible/commands/document_role/role_analysis.py @@ -26,6 +26,7 @@ from docsible.analyzers import analyze_role_complexity from docsible.analyzers.complexity_analyzer.models import ComplexityReport from docsible.analyzers.recommendations import generate_all_recommendations +from docsible.graphs import build_role_execution_graph from docsible.models.recommendation import Recommendation @@ -35,6 +36,7 @@ class RoleAnalysis: complexity_report: ComplexityReport recommendations: list[Recommendation] + execution_graph: Any = None def analyze_role( @@ -57,15 +59,23 @@ def analyze_role( smart defaults) instead of analyzing again Returns: - RoleAnalysis with the complexity report and recommendations + RoleAnalysis with the complexity report, recommendations, and the + one shared execution graph (reused by the complexity metrics and by + any downstream render, so it is never rebuilt per command). """ + execution_graph = build_role_execution_graph(role_info) complexity_report = cached_complexity_report or analyze_role_complexity( role_info, include_patterns=include_patterns, min_confidence=min_confidence, + execution_graph=execution_graph, ) recommendations = generate_all_recommendations(role_path, complexity_report) - return RoleAnalysis(complexity_report=complexity_report, recommendations=recommendations) + return RoleAnalysis( + complexity_report=complexity_report, + recommendations=recommendations, + execution_graph=execution_graph, + ) def render_analyzed_role( @@ -93,6 +103,7 @@ def render_analyzed_role( auto_fix: bool = False, strict_validation: bool = False, playbook_content: str | None = None, + execution_graph: Any | None = None, ) -> Path: """Generate diagrams/dependency matrix and render a role README. @@ -113,7 +124,8 @@ def render_analyzed_role( from docsible.renderers.readme_renderer import ReadmeRenderer analysis_report = analysis.complexity_report - execution_graph = build_role_execution_graph(role_info) + if execution_graph is None: + execution_graph = build_role_execution_graph(role_info) diagrams = generate_mermaid_diagrams( generate_graph=generate_graph, diff --git a/docsible/graphs/role_execution.py b/docsible/graphs/role_execution.py index 0e37387..3142f11 100644 --- a/docsible/graphs/role_execution.py +++ b/docsible/graphs/role_execution.py @@ -168,6 +168,11 @@ def to_dict(self) -> dict[str, Any]: _TASK_FILE_ACTIONS = {"include", "include_tasks", "import_tasks"} _ROLE_ACTIONS = {"include_role", "import_role"} _VARIABLE_PATTERN = re.compile(r"\b([A-Za-z_][A-Za-z0-9_]*)\b") +# One-pass identifier tokenizer for variable-usage edges. Equivalent to the +# previous per-variable `\b\b` search (a variable "matches" iff it +# appears as a whole identifier token in the serialized task), but scans each +# task once instead of once per known variable. +_IDENT_TOKEN_PATTERN = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") _ROLE_PATH_TASK_PREFIX = re.compile(r"^\{\{\s*role_path\s*\}\}/tasks/") @@ -270,9 +275,9 @@ def _module_name(task: dict[str, Any]) -> str: def _add_variable_edges(graph: RoleExecutionGraph, task_id: str, task: dict[str, Any], variables: dict[str, str], source: SourceLocation) -> None: - text = str(task) + referenced = set(_IDENT_TOKEN_PATTERN.findall(str(task))) for name, node_id in variables.items(): - if re.search(rf"\b{re.escape(name)}\b", text): + if name in referenced: graph.add_edge(GraphEdge(EdgeKind.USES_VARIABLE, task_id, node_id, ResolutionStatus.STATIC, source)) diff --git a/tests/graphs/test_role_execution.py b/tests/graphs/test_role_execution.py index de14bb7..26c5837 100644 --- a/tests/graphs/test_role_execution.py +++ b/tests/graphs/test_role_execution.py @@ -119,3 +119,29 @@ def test_loop_control_recorded_in_task_metadata(): assert loop_nodes, "loop_control must be recorded on the task node" assert loop_nodes[0].metadata["loop"] == "loop" assert loop_nodes[0].metadata["loop_control"] == {"loop_var": "entry", "index_var": "i"} + + +def test_uses_variable_edge_survives_tokenizer_rewrite(): + """The (a) optimization replaced a per-variable regex with a one-pass + identifier tokenizer; this guards that a genuinely referenced variable + still yields a uses_variable edge.""" + graph = build_role_execution_graph( + { + "name": "web", + "defaults": [{"file": "main.yml", "data": {"web_port": {"line": 1}}}], + "vars": [], + "handlers": [], + "tasks": [ + { + "file": "main.yml", + "tasks": [{}], + "line_ranges": [(1, 3)], + "mermaid": [ + {"name": "Bind", "ansible.builtin.template": {"port": "{{ web_port }}"}} + ], + } + ], + } + ) + var_edges = [e for e in graph.edges if e.kind is EdgeKind.USES_VARIABLE] + assert [e.target_id for e in var_edges] == ["variable:web:defaults:web_port"] From 828526b7e9293f3f71ad97909a3bae2a7ca1968b Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 20:05:04 +0200 Subject: [PATCH 19/20] =?UTF-8?q?refactor:=20derive=20include=20boundary?= =?UTF-8?q?=20counts=20from=20the=20execution=20graph=20(single=20source?= =?UTF-8?q?=20of=20truth)=20role=5Fanalyzer.analyze=5Frole=5Fcomplexity:?= =?UTF-8?q?=20task=5Fincludes/role=5Fincludes=20are=20now=20derived=20from?= =?UTF-8?q?=20the=20RoleExecutionGraph=20(distinct=20source=20tasks=20of?= =?UTF-8?q?=20its=20include=20edges),=20replacing=20the=20separate=20flatt?= =?UTF-8?q?ened-task=20regex=20that=20never=20matched=20bare=20include:.?= =?UTF-8?q?=20Two=20complexity=20fixtures=20updated=20to=20carry=20a=20pro?= =?UTF-8?q?duction-faithful=20mermaid=20list=20(they=20previously=20had=20?= =?UTF-8?q?only=20processed=20tasks,=20which=20the=20graph=20doesn't=20rea?= =?UTF-8?q?d=20=E2=80=94=20that's=20why=20they=20initially=20broke).=20New?= =?UTF-8?q?=20regression=20test=20locks=20"legacy=20include:=20counted=20+?= =?UTF-8?q?=20graph-authoritative."?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAIMS.md | 19 +++++++ .../analyzers/role_analyzer.py | 54 +++++++++---------- tests/analyzers/complexity/conftest.py | 9 ++++ tests/analyzers/complexity/test_analyzer.py | 29 ++++++++++ tests/test_complexity_analyzer.py | 9 ++++ 5 files changed, 90 insertions(+), 30 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 88421d0..5c6f38d 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -233,6 +233,25 @@ interface is `build_role_execution_graph(role_info)`. file` vs `N task files`). This subsumes finding B (nested roles already give the bounded multi-level view); finding D stays deferred (see Next Graph Milestones). +13. Scale performance of the graph path (found on candidate 8, + `UBUNTU22-CIS`): `_add_variable_edges` now tokenizes each serialized task + once (one identifier-regex pass + set membership) instead of a + per-variable `\b\b` regex scan — graph build 12.7s → 0.3s, edge set + unchanged (1745 nodes / 2510 edges). The `RoleExecutionGraph` is also now + built once per command and threaded through `RoleAnalysis`, + `analyze_role_complexity(execution_graph=...)`, `render_analyzed_role`, + and `_generate_diagrams` (previously rebuilt 2–3×). CIS `analyze` + 41s → 3s; `document --graph` 40s → 3s. +14. The `RoleExecutionGraph` is the single authoritative counter for + include/import boundaries: `task_includes`/`role_includes` in the + complexity report are derived from distinct source tasks of the graph's + include edges, replacing a separate flattened-task regex that never + matched the legacy bare `include:` keyword. Fixes the candidate-9 + contradiction (legacy `include:` role reported `Task Includes: 0` while + its own Execution Routes/diagram showed 20; now 20). Non-legacy roles are + unchanged (verified docker 5, mysql 9, nginx-official 21, os_hardening + 22). Any future boundary count must read from the graph — not add a second + scan — to keep this single source of truth. ### Next Graph Milestones diff --git a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py index 95bb8da..4cbb8cb 100644 --- a/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py +++ b/docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py @@ -162,32 +162,30 @@ def analyze_role_complexity( role_dependencies = len(role_info.get("meta", {}).get("dependencies", [])) collection_dependencies = len(role_info.get("meta", {}).get("collections", [])) - # Count role includes (include_role, import_role) - role_includes = sum( - 1 - for tf in tasks_data - for task in tf.get("tasks", []) - if task.get("module", "") - in [ - "include_role", - "import_role", - "ansible.builtin.include_role", - "ansible.builtin.import_role", - ] - ) + # Build the execution graph once; it is the authoritative source for + # boundary counts and every graph-derived metric below. This replaces a + # second regex scan of the flattened tasks, which historically missed the + # legacy bare `include:` keyword. Count distinct *source* tasks (not edges) + # so one templated include that fans out to several candidate files is + # still counted as the single boundary statement it is. + from docsible.graphs import EdgeKind, NodeKind, ResolutionStatus, build_role_execution_graph - # Count task includes (include_tasks, import_tasks) - task_includes = sum( - 1 - for tf in tasks_data - for task in tf.get("tasks", []) - if task.get("module", "") - in [ - "include_tasks", - "import_tasks", - "ansible.builtin.include_tasks", - "ansible.builtin.import_tasks", - ] + if execution_graph is None: + execution_graph = build_role_execution_graph(role_info) + + task_includes = len( + { + edge.source_id + for edge in execution_graph.edges + if edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE} + } + ) + role_includes = len( + { + edge.source_id + for edge in execution_graph.edges + if edge.kind in {EdgeKind.INCLUDES_ROLE, EdgeKind.IMPORTS_ROLE} + } ) # Calculate max and average tasks per file @@ -207,11 +205,7 @@ def analyze_role_complexity( # Detect inflection points inflection_points = detect_inflection_points(role_info, hotspots) - # Create metrics - from docsible.graphs import EdgeKind, NodeKind, ResolutionStatus, build_role_execution_graph - - if execution_graph is None: - execution_graph = build_role_execution_graph(role_info) + # Create metrics (execution_graph already built above; reuse it). phases = execution_graph.execution_phases() graph_metrics = { "static_reachable_task_files": sum( diff --git a/tests/analyzers/complexity/conftest.py b/tests/analyzers/complexity/conftest.py index adecf5a..b5d9d97 100644 --- a/tests/analyzers/complexity/conftest.py +++ b/tests/analyzers/complexity/conftest.py @@ -140,6 +140,15 @@ def complex_role_info(): {"name": "Another task", "module": "copy"}, {"name": "Final task", "module": "template"}, ], + "mermaid": [ + {"name": "Include common tasks", "include_tasks": "common.yml"}, + {"name": "Import role", "import_role": {"name": "base"}}, + {"name": "Include another role", "ansible.builtin.include_role": {"name": "utils"}}, + {"name": "Import more tasks", "ansible.builtin.import_tasks": "cleanup.yml"}, + {"name": "Regular task", "debug": {}}, + {"name": "Another task", "copy": {}}, + {"name": "Final task", "template": {}}, + ], }, ], "handlers": [ diff --git a/tests/analyzers/complexity/test_analyzer.py b/tests/analyzers/complexity/test_analyzer.py index 44dadf7..f35c2d2 100644 --- a/tests/analyzers/complexity/test_analyzer.py +++ b/tests/analyzers/complexity/test_analyzer.py @@ -98,3 +98,32 @@ def test_analyze_max_and_avg_tasks(): assert report.metrics.task_files == 3 assert report.metrics.max_tasks_per_file == 15 assert report.metrics.avg_tasks_per_file == 10.0 # (10+5+15)/3 + + +def test_task_includes_is_graph_authoritative_and_counts_legacy_include(): + """Regression: boundary counts come from the RoleExecutionGraph, not a + separate flattened-task regex that missed the legacy bare `include:` + keyword (candidate 9 reported 0 despite 20 include boundaries).""" + role_info = { + "name": "legacy", + "defaults": [], + "vars": [], + "handlers": [], + "tasks": [ + { + "file": "main.yml", + "tasks": [ + {"name": "inc", "module": "include", "type": "task", "when": None} + ], + "mermaid": [{"name": "inc", "include": "sub.yml"}], + }, + { + "file": "sub.yml", + "tasks": [{"name": "d", "module": "debug", "type": "task", "when": None}], + "mermaid": [{"name": "d", "debug": {}}], + }, + ], + } + metrics = analyze_role_complexity(role_info).metrics + assert metrics.task_includes == 1 # bare include: counted via the graph + assert metrics.role_includes == 0 diff --git a/tests/test_complexity_analyzer.py b/tests/test_complexity_analyzer.py index ab9cb49..0419ef6 100644 --- a/tests/test_complexity_analyzer.py +++ b/tests/test_complexity_analyzer.py @@ -153,6 +153,15 @@ def create_complex_role_info(): {"name": "Another task", "module": "copy"}, {"name": "Final task", "module": "template"}, ], + "mermaid": [ + {"name": "Include common tasks", "include_tasks": "common.yml"}, + {"name": "Import role", "import_role": {"name": "base"}}, + {"name": "Include another role", "ansible.builtin.include_role": {"name": "utils"}}, + {"name": "Import more tasks", "ansible.builtin.import_tasks": "cleanup.yml"}, + {"name": "Regular task", "debug": {}}, + {"name": "Another task", "copy": {}}, + {"name": "Final task", "template": {}}, + ], }, ], "handlers": [ From 9f1105f853152ec78b96dda9a8c0fc730599daed Mon Sep 17 00:00:00 2001 From: Jier Date: Sun, 13 Sep 2026 20:50:56 +0200 Subject: [PATCH 20/20] update complexity ownership in claims to pick up later --- CLAIMS.md | 72 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/CLAIMS.md b/CLAIMS.md index 5c6f38d..7e801c4 100644 --- a/CLAIMS.md +++ b/CLAIMS.md @@ -253,6 +253,37 @@ interface is `build_role_execution_graph(role_info)`. 22). Any future boundary count must read from the graph — not add a second scan — to keep this single source of truth. +### Complexity ownership: graph vs residual scans + +`analyze_role_complexity` still mixes two kinds of computation. Goal: make the +`RoleExecutionGraph` the authoritative source for as much as is graph-shaped, so +the same fact is never computed twice by two implementations (the +`task_includes`/legacy-`include:` drift was the first instance of this class). + +- Derived from the graph (single source of truth): `task_includes`, + `role_includes`, `static_reachable_task_files`, `dynamic_boundaries`, + `unknown_boundaries`, `external_role_references`, `loop_tasks`, + `notification_edges`, `orphan_task_files`, `conditional_decision_points`. +- Still computed by separate scans of `role_info` (acceptable structural + counts): `total_tasks`, `task_files`, `handlers`, `max_tasks_per_file`, + `avg_tasks_per_file`; meta reads `role_dependencies`, + `collection_dependencies`; and the non-graph analyzers + `external_integrations` (`detect_integrations`), `file_details` + (`analyze_file_complexity`), and the hotspot/inflection detectors. +- Two residual scans are flagged risks, not yet fixed: + - `conditional_tasks` and the graph-derived `conditional_decision_points` + measure the same concept through two implementations. They agree on every + tested role today (CIS: 592 = 592) but can silently drift, exactly like + `task_includes` did before it was made graph-derived. Recommendation: + keep the graph-derived value authoritative and drop or alias the scan. + - `error_handlers` is effectively dead: it counts `task.get("rescue") or + task.get("always")` over the *flattened processed* tasks, but the + flattener emits block/rescue/always as separate rows (with `module`), + never as a `rescue`/`always` key on a task — so it reports **0** even for + a role with ~189 blocks (verified on `UBUNTU22-CIS`). It is both + mis-implemented and a residual scan; the right owner is the graph, which + already walks real block/rescue/always — see Next Graph Milestones #3. + ### Next Graph Milestones 1. Publish a documented JSON graph contract after its node and edge fields are @@ -263,9 +294,19 @@ interface is `build_role_execution_graph(role_info)`. full collection-wide dependency graph, which was assessed as low value: readers almost always want one role's own dependencies, not a map of all roles' relationships). + Status: the primitives already exist — `EXTERNAL_ROLE` nodes, + `INCLUDES_ROLE`/`IMPORTS_ROLE` edges, the `unresolved_external` resolution + state, and a now graph-derived `role_includes` count. What this milestone + adds is (a) resolving a `name`/FQCN reference that matches a sibling + `roles/` into an internal node, (b) honoring `tasks_from` for a + precise entry-point edge, and (c) a thin composition layer that joins the + per-role graphs via those role edges. This is the concrete building block + for the collection milestone and is incremental on the existing model, not + a new subsystem. 3. Add graph projections for blocks, rescue/always, and source-linked variable scopes without claiming static certainty where Ansible defers - resolution. + resolution. Fixing block/rescue/always representation here also repairs + the dead `error_handlers` metric (see Complexity ownership above). 4. Make `graph_visualisation` a renderer adapter over this contract, using NetworkX only for renderer-specific layout work. 5. Extend the pinned external corpus before treating the graph contract as @@ -344,17 +385,28 @@ smaller/synthetic test collection did not surface: The source-only duplication scan is below the original baseline, but remaining duplication is prioritized by ownership and behavior rather than percentage. -1. **Partially resolved.** Role complexity/execution-graph/recommendation - *analysis* is now consolidated in `role_analysis.py` (see Role Execution - Graph, milestone 7) and used identically by `document role`, - `document role --collection`, and `scan collection`. Role-information - *loading* still has one remaining duplicate: `RoleInfoBuilder` alongside - `RoleInfoLoader` (see Known Limitations). -2. Consider a private helper for repeated integration-provider task traversal +1. **Largely resolved.** Role complexity/execution-graph/recommendation + *analysis* is now consolidated in `role_analysis.py` (milestone 7), the + `RoleExecutionGraph` is built once per command and threaded (milestone 13), + and include/role boundary counts are graph-derived (milestone 14). Used + identically by `document role`, `document role --collection`, and + `scan collection`. +2. Collapse the remaining duplicate complexity scans into the graph so a fact + is computed once: `conditional_tasks` (alias/derive from + `conditional_decision_points`) and `error_handlers` (via real + block/rescue/always projection, Next Graph Milestones #3). Until then they + are two implementations of one concept and can drift. +3. Role-information *loading* still has one remaining duplicate: the deprecated + `RoleInfoBuilder` alongside `RoleInfoLoader` (see Known Limitations). + Retire `RoleInfoBuilder` and the deprecated `docsible role` command, and + finish single-path loading, **before** freezing the public JSON graph + contract (Next Graph Milestones #1) so that contract ships against a + de-duplicated, stable surface rather than being revised after the fact. +4. Consider a private helper for repeated integration-provider task traversal after the role-loader migration is complete. -3. Review overlapping renderer model fields only when a concrete rendering +5. Review overlapping renderer model fields only when a concrete rendering change requires them to move together. -4. Remove obsolete duplicate tests and generated fixture backups only after +6. Remove obsolete duplicate tests and generated fixture backups only after confirming they are not test contracts. ## Scope of This Document