Skip to content

Commit 232597b

Browse files
authored
Merge pull request #12 from jier/feature/authorative-complexity-reachability
- error_handlers fixed: was dead (scanned flattened tasks for a rescue/always key that never exists → always 0). Now the graph detects block/rescue/always and sets error_handling on the node; error_handlers counts it. Unit-tested (rescue block → 1); stays 0 for roles genuinely without rescue (CIS, openstack). - Reachability is now a complete, honest partition (the candidate-10 finding): static_reachable + dynamically_reachable + unreachable == task_files. openstack now reads 2 + 13 + 5 = 20 (previously "static 2 / orphan 5" silently hid the 13 dynamic-gated files). orphan_task_files renamed unreachable_task_files, with a README note explaining the tiers (and that files behind an unresolved dynamic boundary stay "unreachable" — the graph still won't guess {{ pkg_mgr }}.yml).
2 parents df03077 + 1d29c74 commit 232597b

12 files changed

Lines changed: 211 additions & 58 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ npx --yes jscpd docsible --pattern "**/*.py"
3434

3535
`jscpd` is informational: its current baseline is 21 clones and 1.30% duplicated lines, not a zero-threshold gate. Update the baseline only after reviewing intentional duplication.
3636

37-
## Verified Baseline (2026-08-27)
37+
## Verified Baseline (2026-09-13)
3838

39-
- `uv run pytest`: 1156 passed, 10 warnings.
40-
- `uv run ruff check .`: 47 findings.
41-
- `uv run mypy docsible`: 3 errors in 2 files.
39+
- `uv run pytest` passes with 1215 tests (3 xpassed).
40+
- `uv run ruff check .` reports no findings.
41+
- `uv run mypy docsible` reports no issues.
4242

4343
Treat these results as a starting point, not permission to introduce additional failures.

CLAIMS.md

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -261,28 +261,39 @@ the same fact is never computed twice by two implementations (the
261261
`task_includes`/legacy-`include:` drift was the first instance of this class).
262262

263263
- Derived from the graph (single source of truth): `task_includes`,
264-
`role_includes`, `static_reachable_task_files`, `dynamic_boundaries`,
265-
`unknown_boundaries`, `external_role_references`, `loop_tasks`,
266-
`notification_edges`, `orphan_task_files`, `conditional_decision_points`.
264+
`role_includes`, `conditional_tasks`, `error_handlers`,
265+
`static_reachable_task_files`, `dynamically_reachable_task_files`,
266+
`unreachable_task_files`, `dynamic_boundaries`, `unknown_boundaries`,
267+
`external_role_references`, `loop_tasks`, `notification_edges`,
268+
`conditional_decision_points`.
267269
- Still computed by separate scans of `role_info` (acceptable structural
268-
counts): `total_tasks`, `task_files`, `handlers`, `max_tasks_per_file`,
269-
`avg_tasks_per_file`; meta reads `role_dependencies`,
270+
counts, not graph facts): `total_tasks`, `task_files`, `handlers`,
271+
`max_tasks_per_file`, `avg_tasks_per_file`; meta reads `role_dependencies`,
270272
`collection_dependencies`; and the non-graph analyzers
271273
`external_integrations` (`detect_integrations`), `file_details`
272274
(`analyze_file_complexity`), and the hotspot/inflection detectors.
273-
- Two residual scans are flagged risks, not yet fixed:
274-
- `conditional_tasks` and the graph-derived `conditional_decision_points`
275-
measure the same concept through two implementations. They agree on every
276-
tested role today (CIS: 592 = 592) but can silently drift, exactly like
277-
`task_includes` did before it was made graph-derived. Recommendation:
278-
keep the graph-derived value authoritative and drop or alias the scan.
279-
- `error_handlers` is effectively dead: it counts `task.get("rescue") or
280-
task.get("always")` over the *flattened processed* tasks, but the
281-
flattener emits block/rescue/always as separate rows (with `module`),
282-
never as a `rescue`/`always` key on a task — so it reports **0** even for
283-
a role with ~189 blocks (verified on `UBUNTU22-CIS`). It is both
284-
mis-implemented and a residual scan; the right owner is the graph, which
285-
already walks real block/rescue/always — see Next Graph Milestones #3.
275+
- Resolved this milestone:
276+
- `conditional_tasks` no longer has its own flattened-task scan — it is now
277+
derived from the same graph pass as `conditional_decision_points`, so the
278+
two cannot drift (previously the first duplicate-scan class instance after
279+
`task_includes`).
280+
- `error_handlers` was dead (it scanned flattened tasks for a `rescue`/
281+
`always` key that is never there, so always 0). It is now graph-derived
282+
from real block/rescue/always detection; a rescue block counts as 1
283+
(unit-tested), and roles without rescue/always correctly stay 0.
284+
- Reachability is now a complete, honest partition. Previously a file reached
285+
only through a dynamic boundary was counted as neither "static reachable"
286+
nor "orphan" and silently vanished (openstack `ansible-hardening` showed
287+
"static 2 / orphan 5" for a 20-file role). Now `static_reachable +
288+
dynamically_reachable + unreachable == task_files` (openstack: 2 + 13 + 5
289+
= 20), and the misleading "orphan" field is renamed `unreachable_task_files`
290+
(no inbound edge from any *resolved* boundary), with the graph summary and
291+
README explaining the three tiers.
292+
- Still open (structural depth, not metrics): the graph flags that a block has
293+
rescue/always but does not yet model the block/rescue/always control flow as
294+
first-class nodes/edges, and files reachable only through an *unresolved*
295+
dynamic boundary (e.g. `{{ pkg_mgr }}.yml`) still read as `unreachable`
296+
because the graph deliberately does not guess which concrete file runs.
286297

287298
### Next Graph Milestones
288299

@@ -303,10 +314,11 @@ the same fact is never computed twice by two implementations (the
303314
per-role graphs via those role edges. This is the concrete building block
304315
for the collection milestone and is incremental on the existing model, not
305316
a new subsystem.
306-
3. Add graph projections for blocks, rescue/always, and source-linked
307-
variable scopes without claiming static certainty where Ansible defers
308-
resolution. Fixing block/rescue/always representation here also repairs
309-
the dead `error_handlers` metric (see Complexity ownership above).
317+
3. Model block/rescue/always as first-class graph structure (nodes/edges), and
318+
add source-linked variable scopes, without claiming static certainty where
319+
Ansible defers resolution. (The `error_handlers` metric is already
320+
graph-derived from real rescue/always detection; what remains is exposing
321+
the block control flow itself, not just the count.)
310322
4. Make `graph_visualisation` a renderer adapter over this contract, using
311323
NetworkX only for renderer-specific layout work.
312324
5. Extend the pinned external corpus before treating the graph contract as
@@ -391,11 +403,13 @@ duplication is prioritized by ownership and behavior rather than percentage.
391403
and include/role boundary counts are graph-derived (milestone 14). Used
392404
identically by `document role`, `document role --collection`, and
393405
`scan collection`.
394-
2. Collapse the remaining duplicate complexity scans into the graph so a fact
395-
is computed once: `conditional_tasks` (alias/derive from
396-
`conditional_decision_points`) and `error_handlers` (via real
397-
block/rescue/always projection, Next Graph Milestones #3). Until then they
398-
are two implementations of one concept and can drift.
406+
2. **Resolved.** The duplicate complexity scans are collapsed into the graph:
407+
`conditional_tasks` is now derived from the same graph pass as
408+
`conditional_decision_points` (one source, cannot drift), and
409+
`error_handlers` is graph-derived from real block/rescue/always detection
410+
(was always 0). Reachability was also made a complete partition
411+
(`static + dynamic-only + unreachable == task_files`) with the misleading
412+
`orphan` renamed to `unreachable`.
399413
3. Role-information *loading* still has one remaining duplicate: the deprecated
400414
`RoleInfoBuilder` alongside `RoleInfoLoader` (see Known Limitations).
401415
Retire `RoleInfoBuilder` and the deprecated `docsible role` command, and

README.md

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ Project home: https://github.com/jier/docsible
6262
- Preset system — four built-in presets covering personal, team, enterprise, and consulting use cases
6363
- Suppression system — silence false-positive recommendations with audit trail and optional expiry
6464
- Interactive setup wizard (`docsible init`) with optional CI/CD workflow generation
65+
- Source-backed `RoleExecutionGraph` — typed include/import, cross-role, notification and variable edges, each with static / dynamic / unknown resolution and a source location
66+
- README "Execution Routes" + "Execution Graph Summary" — role entry point, statically vs dynamically reachable and unreachable files, and dynamic boundaries, instead of filesystem-order phases
67+
- Collections get per-role documentation plus a collection-level complexity overview and a role index sorted by complexity
68+
- Machine-readable `--output-format json` exposes the complexity metrics and the serialized execution graph for CI and downstream renderers
6569

6670
## Installation
6771

@@ -125,7 +129,8 @@ docsible scan collection . --fail-on warning --output-format json
125129

126130
### `--output-format json`
127131

128-
Use `--output-format json` with `docsible analyze role` for machine-readable output:
132+
Use `--output-format json` with `docsible analyze role` (also supported by
133+
`validate role` and `document role`) for machine-readable output:
129134

130135
```bash
131136
docsible analyze role --role . --output-format json
@@ -137,13 +142,26 @@ Output schema:
137142
{
138143
"role": "my-role",
139144
"findings": [
140-
{ "severity": "WARNING", "message": "No example playbook found", "category": "documentation" }
145+
{ "severity": "warning", "message": "No example playbook found", "category": "documentation" }
141146
],
142-
"summary": { "total": 3, "critical": 0, "warning": 2, "info": 1 },
143-
"truncated": false
147+
"summary": { "total": 3, "shown": 3, "critical": 0, "warning": 2, "info": 1 },
148+
"truncated": false,
149+
"complexity": {
150+
"total_tasks": 3, "task_files": 1, "handlers": 0,
151+
"task_includes": 0, "conditional_tasks": 1, "error_handlers": 0,
152+
"static_reachable_task_files": 1, "dynamically_reachable_task_files": 0,
153+
"unreachable_task_files": 0, "dynamic_boundaries": 0, "loop_tasks": 0,
154+
"notification_edges": 0, "collection_dependencies": 0
155+
},
156+
"execution_graph": { "role_id": "role:my-role", "nodes": ["..."], "edges": ["..."] }
144157
}
145158
```
146159

160+
`complexity` is derived from the `RoleExecutionGraph` (the single source of
161+
truth for boundary, loop, notification and reachability counts), and
162+
`execution_graph` is the full serialized node/edge model. `truncated` applies
163+
to `findings`; the graph itself is never truncated.
164+
147165
### Ready-to-use CI examples
148166

149167
See [`examples/ci_pipeline/`](examples/ci_pipeline/) for complete, ready-to-use configurations:

docsible/analyzers/complexity_analyzer/analyzers/role_analyzer.py

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -145,18 +145,10 @@ def analyze_role_complexity(
145145
# Count handlers
146146
handlers = len(role_info.get("handlers", []))
147147

148-
# Count conditional tasks
149-
conditional_tasks = sum(
150-
1 for tf in tasks_data for task in tf.get("tasks", []) if task.get("when")
151-
)
152-
153-
# Count tasks with error handling (rescue or always blocks)
154-
error_handlers = sum(
155-
1
156-
for tf in tasks_data
157-
for task in tf.get("tasks", [])
158-
if task.get("rescue") or task.get("always")
159-
)
148+
# conditional_tasks and error_handlers are derived from the
149+
# RoleExecutionGraph below (single source of truth), instead of a second
150+
# flattened-task scan. The old error_handlers scan read a `rescue`/`always`
151+
# key that flattened tasks never carry, so it was always 0.
160152

161153
# Count role dependencies (from meta/main.yml)
162154
role_dependencies = len(role_info.get("meta", {}).get("dependencies", []))
@@ -207,10 +199,16 @@ def analyze_role_complexity(
207199

208200
# Create metrics (execution_graph already built above; reuse it).
209201
phases = execution_graph.execution_phases()
202+
task_nodes = [node for node in execution_graph.nodes.values() if node.kind is NodeKind.TASK]
210203
graph_metrics = {
204+
"conditional_tasks": sum("condition" in node.metadata for node in task_nodes),
205+
"error_handlers": sum("error_handling" in node.metadata for node in task_nodes),
211206
"static_reachable_task_files": sum(
212207
phase["kind"] in {"entrypoint", "static", "conditional"} for phase in phases
213208
),
209+
"dynamically_reachable_task_files": sum(
210+
phase["kind"] == "dynamic" for phase in phases
211+
),
214212
"dynamic_boundaries": sum(
215213
edge.resolution is ResolutionStatus.DYNAMIC
216214
and edge.kind in {EdgeKind.INCLUDES_TASK_FILE, EdgeKind.IMPORTS_TASK_FILE, EdgeKind.INCLUDES_ROLE, EdgeKind.IMPORTS_ROLE}
@@ -232,7 +230,7 @@ def analyze_role_complexity(
232230
edge.kind is EdgeKind.NOTIFIES_HANDLER and edge.target_id is not None
233231
for edge in execution_graph.edges
234232
),
235-
"orphan_task_files": sum(phase["kind"] == "unreachable" for phase in phases),
233+
"unreachable_task_files": sum(phase["kind"] == "unreachable" for phase in phases),
236234
"conditional_decision_points": sum(
237235
node.kind is NodeKind.TASK and "condition" in node.metadata
238236
for node in execution_graph.nodes.values()
@@ -242,8 +240,6 @@ def analyze_role_complexity(
242240
total_tasks=total_tasks,
243241
task_files=task_files,
244242
handlers=handlers,
245-
conditional_tasks=conditional_tasks,
246-
error_handlers=error_handlers,
247243
role_dependencies=role_dependencies,
248244
collection_dependencies=collection_dependencies,
249245
role_includes=role_includes,

docsible/analyzers/complexity_analyzer/models.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,20 @@ class ComplexityMetrics(BaseModel):
6565
task_includes: int = Field(default=0, description="include_tasks/import_tasks count")
6666

6767
# Execution graph metrics (source-backed relationships, not runtime claims)
68-
static_reachable_task_files: int = Field(default=0)
68+
static_reachable_task_files: int = Field(
69+
default=0, description="Files reachable via static/conditional boundaries only"
70+
)
71+
dynamically_reachable_task_files: int = Field(
72+
default=0, description="Files reachable only through an unresolved dynamic boundary"
73+
)
6974
dynamic_boundaries: int = Field(default=0)
7075
unknown_boundaries: int = Field(default=0)
7176
external_role_references: int = Field(default=0)
7277
loop_tasks: int = Field(default=0)
7378
notification_edges: int = Field(default=0)
74-
orphan_task_files: int = Field(default=0)
79+
unreachable_task_files: int = Field(
80+
default=0, description="Files with no inbound boundary from any resolved edge"
81+
)
7582
conditional_decision_points: int = Field(default=0)
7683

7784
# External integrations

docsible/formatters/text/dry_run.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,9 @@ def _format_complexity(self, analysis_report, role_info: dict) -> str:
119119
lines.append(
120120
" Execution graph: "
121121
f"{metrics.static_reachable_task_files} static files, "
122+
f"{metrics.dynamically_reachable_task_files} dynamic-only, "
122123
f"{metrics.dynamic_boundaries} dynamic boundaries, "
123-
f"{metrics.orphan_task_files} orphans"
124+
f"{metrics.unreachable_task_files} unreachable"
124125
)
125126

126127
return "\n".join(lines)

docsible/graphs/role_execution.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,8 @@ def _add_tasks(graph: RoleExecutionGraph, role_name: str, task_file: dict[str, A
247247
metadata["loop"] = loop
248248
if loop_control := extract_loop_control(task):
249249
metadata["loop_control"] = loop_control
250+
if error_handling := _error_handling(task):
251+
metadata["error_handling"] = error_handling
250252
graph.add_node(GraphNode(task_id, NodeKind.TASK, str(task.get("name", "Unnamed")), source, metadata))
251253
graph.add_edge(GraphEdge(EdgeKind.CONTAINS, file_ids[file_name], task_id, ResolutionStatus.STATIC, source))
252254
_add_variable_edges(graph, task_id, task, variables, source)
@@ -365,3 +367,16 @@ def _loop(task: dict[str, Any]) -> str | None:
365367
if "loop" in task:
366368
return "loop"
367369
return next((key for key in task if key.startswith("with_")), None)
370+
371+
372+
def _error_handling(task: dict[str, Any]) -> str | None:
373+
"""Return the block error-handling shape ('rescue'/'always'/both) if any."""
374+
has_rescue = isinstance(task.get("rescue"), list)
375+
has_always = isinstance(task.get("always"), list)
376+
if has_rescue and has_always:
377+
return "rescue + always"
378+
if has_rescue:
379+
return "rescue"
380+
if has_always:
381+
return "always"
382+
return None

docsible/templates/role/sections/adaptive_diagrams.jinja2

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,17 @@ This role contains **{{ complexity_report.metrics.total_tasks }} tasks** across
6464
### Execution Graph Summary
6565
- Collection dependencies: {{ complexity_report.metrics.collection_dependencies }}
6666
- Conditional decision points: {{ complexity_report.metrics.conditional_decision_points }}
67+
- Error handlers (blocks with rescue/always): {{ complexity_report.metrics.error_handlers }}
6768
- Statically reachable task files: {{ complexity_report.metrics.static_reachable_task_files }}
69+
- Dynamically reachable task files: {{ complexity_report.metrics.dynamically_reachable_task_files }}
70+
- Unreachable task files: {{ complexity_report.metrics.unreachable_task_files }}
6871
- Dynamic boundaries: {{ complexity_report.metrics.dynamic_boundaries }}
6972
- Unknown boundaries: {{ complexity_report.metrics.unknown_boundaries }}
7073
- Handler notification edges: {{ complexity_report.metrics.notification_edges }}
7174
- Loop-bearing tasks: {{ complexity_report.metrics.loop_tasks }}
72-
- Orphan task files: {{ complexity_report.metrics.orphan_task_files }}
75+
76+
_Dynamically reachable files are reached only through a boundary whose target is
77+
templated; unreachable files have no inbound edge from any resolved boundary._
7378

7479
{% if architecture_diagram %}
7580
### Component Architecture

tests/analyzers/complexity/test_analyzer.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ def test_analyze_conditional_percentage():
6060
{"name": "Task 3", "module": "debug"}, # No condition
6161
{"name": "Task 4", "module": "debug"}, # No condition
6262
],
63+
"mermaid": [
64+
{"name": "Task 1", "debug": {}, "when": "condition1"},
65+
{"name": "Task 2", "debug": {}, "when": "condition2"},
66+
{"name": "Task 3", "debug": {}},
67+
{"name": "Task 4", "debug": {}},
68+
],
6369
}
6470
],
6571
"handlers": [],
@@ -127,3 +133,31 @@ def test_task_includes_is_graph_authoritative_and_counts_legacy_include():
127133
metrics = analyze_role_complexity(role_info).metrics
128134
assert metrics.task_includes == 1 # bare include: counted via the graph
129135
assert metrics.role_includes == 0
136+
137+
138+
def test_error_handlers_is_graph_derived_from_rescue_blocks():
139+
"""Regression: error_handlers used to scan flattened tasks for a
140+
`rescue`/`always` key they never carry (always 0). Now derived from the
141+
execution graph's block/rescue detection."""
142+
role_info = {
143+
"name": "guarded",
144+
"defaults": [],
145+
"vars": [],
146+
"handlers": [],
147+
"meta": {"dependencies": []},
148+
"tasks": [
149+
{
150+
"file": "main.yml",
151+
"tasks": [{"name": "Guarded", "module": "block"}],
152+
"mermaid": [
153+
{
154+
"name": "Guarded",
155+
"block": [{"name": "Try", "debug": {}}],
156+
"rescue": [{"name": "Fallback", "debug": {}}],
157+
}
158+
],
159+
}
160+
],
161+
}
162+
metrics = analyze_role_complexity(role_info).metrics
163+
assert metrics.error_handlers == 1

tests/defaults/test_smart_defaults_cli.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ def test_complex_role_gets_graphs_by_default(self, complex_role_fixture, tmp_pat
4949
# Complex role should have visualization enabled (smart default)
5050
content = output_file.read_text()
5151

52-
# Complex roles may use either Mermaid diagrams OR execution phases
52+
# Complex roles may use either Mermaid diagrams OR execution routes
5353
has_mermaid = "```mermaid" in content
54-
has_execution_phases = "Execution Phases" in content
54+
has_execution_routes = "Execution Routes" in content
5555
has_architecture = "Architecture Overview" in content
5656

57-
assert has_mermaid or (has_execution_phases and has_architecture), \
58-
f"Complex role should have visualization (mermaid: {has_mermaid}, phases: {has_execution_phases}, arch: {has_architecture})"
57+
assert has_mermaid or (has_execution_routes and has_architecture), \
58+
f"Complex role should have visualization (mermaid: {has_mermaid}, routes: {has_execution_routes}, arch: {has_architecture})"
5959

6060
def test_user_override_respected(self, simple_role, tmp_path):
6161
"""User --graph flag should override smart default."""

0 commit comments

Comments
 (0)