docs(128): post-delivery quality review (#133)

* refactor(128): simplify code per /simplify review

Production (scripts/extract-infographic-data.py):
- Move datetime import to module level
- Add module-level constants: _QUALIFYING_SEVERITIES, _SEVERITY_FIELD_PREFERENCE,
  _TIER_SOURCE_LABEL
- Promote duplicated severity extractor closures to _canonical_severity
- Promote _extract_composite_score and _extract_description to module level
- Remove dead parameter threats_content from _build_executive_architecture_payload
- Delete _SEV_RANK local dict, use imported _SEVERITY_ORDINAL directly
- Delete task-reference and step-enumeration comments (F-128, T011, --- Step 1 ---)
- Trim verbose helper docstrings to single-line PEP 257 summaries

Tests (5 files):
- Remove T0NN task references, F-128 banners, and point-in-time gate notes
- Strip # Step 1..6 narrative comments in test bodies
- Trim oversized module docstrings to intent-only
- Apply @pytest.mark.slow to typst-compiling tests
- Consolidate 5 identical subprocess runs in test_existing_image_flags_unchanged
  into a module-scoped agentic_app_report_typst fixture (5x subprocess to 1x)

Infrastructure (pyproject.toml):
- Register slow pytest marker so `pytest -m "not slow"` enables fast iteration

Verification:
- Full suite: 39 tests pass in ~20s
- Fast suite: pytest -m "not slow" runs 32 tests in ~5s (4x speedup)
- Production smoke test against agentic-app sample produces identical output

Co-Authored-By: Claude <noreply@anthropic.com>

* docs(128): add docstrings per docs-lint

Add single-line summaries to two promoted helpers (_extract_composite_score,
_extract_description) that were extracted from closures during the simplify
refactor and lacked documentation.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs(128): update CHANGELOG

Append Feature 128 (Executive Threat Architecture Infographic) entry to
## [Unreleased] / ### Added. Describes the new template, the PDF placement
rationale, the `exec` alias, `all` expansion inclusion, the pytest bootstrap,
and the backward-compatibility guarantee via committed .baseline PDFs.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs(128): review KB entries

Add KB-027 capturing the magnitude of the post-delivery simplify pass on
F-128: 31% reduction in changed-line count (660 deletions, 216 insertions
against 2,139 new lines), dominated by narrative-comment sprawl in the
generated test modules. Recommends treating /aod.document simplify as a
mandatory cleanup gate rather than optional polish.

Bump entry count 26 to 27.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: davidmatousek

CHANGELOG.md

@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- **Executive Threat Architecture Infographic** (Feature 128) — New `/tachi.infographic --template executive-architecture` (alias: `exec`) generates a layered architecture diagram with Critical/High finding callouts, designed for CISO-level readers. In the compiled PDF security report the new page lands immediately after the Executive Summary (pages 2–3 area) so executives see the visual threat narrative within their first-glance window. Included in the `all` shorthand expansion alongside the existing five templates. Backward compatible — example PDFs without a generated `threat-executive-architecture.jpg` render byte-identical to the pre-F-128 baseline. Ships with tachi's first project-level pytest harness (`pyproject.toml`, `requirements-dev.txt`, `tests/`) and five committed `.baseline` PDFs guarding backward compatibility against silent regressions.
 - **Architecture Lifecycle Command** (Feature 120) — `/tachi.architecture` now tracks versions with YAML frontmatter (version, date, description, SHA-256 checksum), archives previous versions to `.archive/v{N}/`, and supports guided updates through change categories. `/tachi.threat-model` automatically snapshots the architecture file into each timestamped output folder for full traceability. Backward compatible with existing architecture files.
 
 ### Changed

docs/INSTITUTIONAL_KNOWLEDGE.md

@@ -5,7 +5,7 @@
 **Created**: {{PROJECT_START_DATE}}
 **Last Updated**: 2026-04-10
 
-**Entry Count**: 26 / 20 (KB System Upgrade triggers at 20 — schedule review)
+**Entry Count**: 27 / 20 (KB System Upgrade triggers at 20 — schedule review)
 **Last Review**: 2026-03-30
 **Status**: ✅ Manual mode (file-based)
 
@@ -548,6 +548,29 @@ Captured during structured delivery retrospective. Smooth sailing — everything
 
 ---
 
+### KB-027: Post-Delivery Simplify Pass Commonly Finds Narrative-Comment Sprawl in Generated Tests
+
+**Date**: 2026-04-10
+**Category**: Process / Tooling
+**Source**: Feature 128 /aod.document simplify review
+**Severity**: Informational
+
+**Problem**: Feature 128 shipped 2,139 lines of new Python (two extraction-script additions plus six test modules bootstrapping the pytest harness). The post-delivery `/aod.document` simplify review deleted 660 lines and rewrote 216 — a 31% reduction in changed-line count — with almost all of that coming from the test modules. The cleanup was dominated by three repeating anti-patterns: (1) module-level docstrings narrating the task-gating story ("Note on the test-first gate (T007)"), (2) per-function step-enumeration comments (`# Step 1:`, `# Step 2:`), and (3) inline references to task IDs (T023, T024, T029, T033) and review-artifact paths that will not make sense after the feature branch is merged.
+
+**Root Cause**: Test generation during feature build captures the implementer's working-memory narrative — which tasks are gating which tests, which steps run in which order, which review findings shaped which assertion — and emits it as comments and docstrings. This information is useful *during* implementation (while the author is still holding the whole plan in their head) but becomes stale the moment the feature merges. The comments then linger as archaeological debris in the test modules, reducing signal density and creating misleading context for future maintainers.
+
+**Solution**: Keep the `/aod.document` simplify step in the lifecycle as a mandatory cleanup gate, not an optional polish. The `/simplify` skill's three-agent review (reuse, quality, efficiency) reliably catches these patterns when pointed at the changed files. For F-128 the pass also caught a dead parameter, two duplicated closures, a hand-rolled severity dict that duplicated an imported constant, and 5× redundant subprocess runs in a parametrized test that collapsed cleanly into a module-scoped fixture.
+
+**Result**: Production code became smaller, tighter, and free of task-reference artifacts. The pytest harness gained a `slow` marker registration enabling `pytest -m "not slow"` to run in ~5 seconds instead of ~20 (4× faster local iteration). Full suite still passes (39/39) and the smoke test against the agentic-app sample produces byte-identical output to the pre-refactor run.
+
+**When to Apply**: Every feature that ships new test modules or non-trivial Python helpers. Expect a post-delivery simplify pass to remove 20–30% of changed-line count through comment and duplication cleanup alone. Allocate time for the `/aod.document` stage rather than treating it as optional.
+
+**Tags**: #retrospective #process #simplify #test-quality #comments #tooling #aod-document
+
+**Quality Score**: 8/10
+
+---
+
 ## Bug Fixes
 
 *No entries yet. Use `/kb-create` to add the first bug fix.*

pyproject.toml

@@ -13,3 +13,6 @@ testpaths = ["tests"]
 python_files = ["test_*.py"]
 python_functions = ["test_*"]
 addopts = "-ra --strict-markers"
+markers = [
+    "slow: tests that invoke typst compile or other slow external subprocesses",
+]

scripts/extract-infographic-data.py

@@ -20,6 +20,7 @@
 import math
 import re
 import sys
+from datetime import datetime, timezone
 from pathlib import Path
 
 sys.path.insert(0, str(Path(__file__).resolve().parent))
@@ -62,9 +63,20 @@
     "Note": "#6B7280",
 }
 
-# Alias for backward compatibility within this script
 _SEVERITY_ORDINAL = SEVERITY_ORDINAL
 
+_QUALIFYING_SEVERITIES = frozenset({"Critical", "High"})
+_SEVERITY_FIELD_PREFERENCE = ("severity", "residual_severity", "risk_level")
+_TIER_SOURCE_LABEL = {1: "compensating-controls", 2: "risk-scores", 3: "threats"}
+
+
+def _canonical_severity(finding):
+    for key in _SEVERITY_FIELD_PREFERENCE:
+        val = finding.get(key)
+        if val:
+            return str(val).strip().title()
+    return ""
+
 
 # =============================================================================
 # T009: Largest Remainder Method
@@ -655,23 +667,8 @@ def _compute_trust_zones(scope_data):
     return zones
 
 
-# =============================================================================
-# F-128: Executive Architecture Helpers
-# =============================================================================
-
 def _normalize_component_name(name):
-    """Normalize a component name for case-insensitive, punctuation-insensitive matching.
-
-    Strips whitespace, lowercases, and removes hyphens, underscores, and spaces so that
-    "API Gateway", "api-gateway", "api_gateway", and "apigateway" all collapse to
-    "apigateway". Used as the matching key for layer-finding association.
-
-    Args:
-        name: Component name string (may be None).
-
-    Returns:
-        Normalized string. Empty string if input is None or empty.
-    """
+    """Collapse component names so "API Gateway", "api-gateway", "api_gateway" all match."""
     if not name:
         return ""
     return (
@@ -684,24 +681,11 @@ def _normalize_component_name(name):
 
 
 def _compute_dfd_type_layers(scope_data):
-    """Group Section 1 components by DFD type for the executive-architecture fallback.
-
-    Used when no trust zones are defined in Section 2. Each unique component `type`
-    becomes an ArchitecturalLayer, with components listed alphabetically within the
-    layer. Layers are sorted alphabetically by type name.
-
-    Args:
-        scope_data: Dict from parse_scope_data().
-
-    Returns:
-        List of layer dicts (name, position, components, component_count, source_kind)
-        or None if scope_data has no components at all.
-    """
+    """Fallback layer derivation: group Section 1 components by DFD type when no trust zones exist."""
     components = scope_data.get("components", [])
     if not components:
         return None
 
-    # Group component names by type
     by_type = {}
     for comp in components:
         ctype = (comp.get("type") or "").strip()
@@ -713,7 +697,6 @@ def _compute_dfd_type_layers(scope_data):
     if not by_type:
         return None
 
-    # Build layer dicts sorted alphabetically by type name
     layers = []
     for position, ctype in enumerate(sorted(by_type.keys())):
         comp_names = sorted(by_type[ctype])
@@ -730,144 +713,96 @@ def _compute_dfd_type_layers(scope_data):
     return layers if layers else None
 
 
-def _select_critical_high_callouts(findings, layers):
-    """Filter Critical/High findings and select one callout per layer.
+def _extract_composite_score(finding):
+    """Prefer composite_score, fall back to residual_score; return float or None."""
+    for key in ("composite_score", "residual_score"):
+        raw = finding.get(key)
+        if raw is None or raw == "":
+            continue
+        try:
+            return float(raw)
+        except (TypeError, ValueError):
+            continue
+    return None
 
-    For each layer:
-      1. Filter findings whose normalized component is in the layer's normalized
-         component set, AND whose severity is Critical or High.
-      2. Sort by:
-         a. severity descending (Critical before High)
-         b. composite_score descending (None treated as 0.0)
-         c. finding_id ascending (lexicographic tie-break)
-      3. Select the first element as the layer's callout.
 
-    Orphaned findings (those whose component matches no layer) are dropped.
+def _extract_description(finding):
+    """Prefer description, fall back to threat, fall back to mitigation."""
+    for key in ("description", "threat", "mitigation"):
+        val = finding.get(key)
+        if val:
+            return str(val).strip()
+    return ""
 
-    Args:
-        findings: List of finding dicts from parse_*_findings(). Expected keys:
-            `id`, `component`, `threat` (or `description`), `severity` or `risk_level`,
-            optional `composite_score`, `residual_score`.
-        layers: List of layer dicts (name, components, ...).
 
-    Returns:
-        List of callout dicts matching the ThreatCallout schema in data-model.md.
-        One callout maximum per layer; layers with no qualifying findings yield no callout.
+def _select_critical_high_callouts(findings, layers):
+    """Select one Critical/High callout per layer.
+
+    Sort order per layer: severity desc, composite score desc, finding id asc.
+    Findings whose component matches no layer are dropped.
     """
-    # Build a lookup: normalized component name -> (layer_name, layer_index)
     comp_to_layer = {}
     for idx, layer in enumerate(layers):
         for comp in layer.get("components", []):
             key = _normalize_component_name(comp)
             if key and key not in comp_to_layer:
                 comp_to_layer[key] = (layer["name"], idx)
 
-    # Severity ordering for sort: Critical=2, High=1 (others excluded by filter)
-    _SEV_RANK = {"critical": 2, "high": 1}
-
-    def _extract_severity(finding):
-        """Return canonical severity label ('Critical', 'High') or empty string."""
-        # Tier 2/1 use `severity` / `residual_severity`; Tier 3 uses `risk_level`.
-        for key in ("severity", "residual_severity", "risk_level"):
-            val = finding.get(key)
-            if val:
-                return str(val).strip().title()
-        return ""
-
-    def _extract_composite_score(finding):
-        """Return composite score as float or None."""
-        for key in ("composite_score", "residual_score"):
-            raw = finding.get(key)
-            if raw is None or raw == "":
-                continue
-            try:
-                return float(raw)
-            except (TypeError, ValueError):
-                continue
-        return None
-
-    def _extract_description(finding):
-        """Return the narrative description for the callout."""
-        for key in ("description", "threat", "mitigation"):
-            val = finding.get(key)
-            if val:
-                return str(val).strip()
-        return ""
-
-    # Group qualifying findings by layer
     per_layer = {layer["name"]: [] for layer in layers}
     for finding in findings:
-        severity = _extract_severity(finding)
-        if severity not in ("Critical", "High"):
+        severity = _canonical_severity(finding)
+        if severity not in _QUALIFYING_SEVERITIES:
             continue
         component = finding.get("component") or ""
         key = _normalize_component_name(component)
         if not key or key not in comp_to_layer:
-            # Orphaned finding — drop per architect L-1 observation
             continue
         layer_name, _idx = comp_to_layer[key]
         per_layer[layer_name].append(finding)
 
-    # Select the top finding per layer using the tie-break rule
     callouts = []
     for layer in layers:
         layer_findings = per_layer.get(layer["name"], [])
         if not layer_findings:
             continue
 
         def _sort_key(f):
-            severity = _extract_severity(f)
+            severity = _canonical_severity(f)
             composite = _extract_composite_score(f)
             return (
-                -_SEV_RANK.get(severity.lower(), 0),          # severity desc
-                -(composite if composite is not None else 0.0),  # score desc
-                f.get("id", ""),                               # id asc
+                -_SEVERITY_ORDINAL.get(severity, 0),
+                -(composite if composite is not None else 0.0),
+                f.get("id", ""),
             )
 
         layer_findings.sort(key=_sort_key)
         top = layer_findings[0]
-        composite = _extract_composite_score(top)
         callouts.append({
             "layer_name": layer["name"],
             "finding_id": top.get("id", ""),
-            "severity": _extract_severity(top),
+            "severity": _canonical_severity(top),
             "raw_description": _extract_description(top),
-            "composite_score": composite,
+            "composite_score": _extract_composite_score(top),
             "affected_component": (top.get("component") or None),
         })
 
     return callouts
 
 
-def _build_executive_architecture_payload(threats_content, tier, findings, scope_data, source_file):
-    """Assemble the ExecutiveArchitecturePayload dict for the executive-architecture template.
-
-    Handles layer derivation (trust zones → DFD type fallback), Critical/High filtering,
-    per-layer callout selection, metadata assembly, and the skip_image flag.
-
-    Args:
-        threats_content: Full text of threats.md (unused; reserved for future use).
-        tier: Tier source label (one of "compensating-controls", "risk-scores", "threats").
-        findings: List of finding dicts from extract_severity().
-        scope_data: Dict from parse_scope_data().
-        source_file: Path-like to the source threats.md file.
+def _build_executive_architecture_payload(tier, findings, scope_data, source_file):
+    """Assemble the ExecutiveArchitecturePayload.
 
-    Returns:
-        ExecutiveArchitecturePayload dict, OR a dict {"error": "no_scope_data"} when
-        neither trust zones nor DFD-type-grouped components can be derived from
-        scope_data. The caller translates the error dict into exit code 2.
+    Derives layers from trust zones (preferred) or falls back to grouping components by
+    DFD type. Returns ``{"error": "no_scope_data"}`` when neither source yields layers;
+    the caller translates that into exit code 2.
     """
-    from datetime import datetime, timezone
-
-    # --- Step 1: Derive architectural layers ---
     trust_zones = _compute_trust_zones(scope_data)
     fallback_used = False
     layers = []
 
     if trust_zones:
-        # Trust zones returned sorted by trust level ASCENDING (trusted first).
-        # The executive-architecture view wants UNTRUSTED at top (position 0 = most
-        # exposed), so we reverse the order.
+        # Trust zones arrive sorted trusted-first; the executive view places the most
+        # exposed layer at position 0, so reverse into untrusted-first order.
         ordered = list(reversed(trust_zones))
         for position, zone in enumerate(ordered):
             comp_names = list(zone.get("components") or [])
@@ -886,35 +821,24 @@ def _build_executive_architecture_payload(threats_content, tier, findings, scope
             fallback_used = True
             layers = dfd_layers
 
-    # Drop any layers that ended up with zero components
     layers = [layer for layer in layers if layer.get("component_count", 0) > 0]
 
     if not layers:
         return {"error": "no_scope_data"}
 
-    # --- Step 2: Filter findings to Critical/High ---
-    def _severity_of(f):
-        for key in ("severity", "residual_severity", "risk_level"):
-            val = f.get(key)
-            if val:
-                return str(val).strip().title()
-        return ""
-
     critical_count = 0
     high_count = 0
     for f in findings:
-        sev = _severity_of(f)
+        sev = _canonical_severity(f)
         if sev == "Critical":
             critical_count += 1
         elif sev == "High":
             high_count += 1
     total_qualifying = critical_count + high_count
 
-    # --- Step 3: Per-layer callout selection ---
     callouts = _select_critical_high_callouts(findings, layers)
     total_after_layer_dedup = len(callouts)
 
-    # --- Step 4: Metadata assembly ---
     skip_image = (total_qualifying == 0)
     generation_timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ")
     metadata = {
@@ -1529,15 +1453,9 @@ def main():
     # Extract severity, findings, and cc_data
     severity, findings, cc_data = extract_severity(tier, threats_content, rs_content, cc_content)
 
-    # F-128 T011: executive-architecture branch (early exit — unique payload shape)
     if args.template == "executive-architecture":
-        # Map integer tier to string tier_source label per data-model.md PayloadMetadata
-        _TIER_SOURCE_LABEL = {1: "compensating-controls", 2: "risk-scores", 3: "threats"}
-        tier_source = _TIER_SOURCE_LABEL.get(tier, "threats")
-
         payload = _build_executive_architecture_payload(
-            threats_content=threats_content,
-            tier=tier_source,
+            tier=_TIER_SOURCE_LABEL.get(tier, "threats"),
             findings=findings,
             scope_data=scope,
             source_file=(target_dir / "threats.md"),
@@ -1564,7 +1482,6 @@ def main():
             file=sys.stderr,
         )
         sys.exit(EXIT_SUCCESS)
-    # End F-128 T011 early exit
 
     # Compute severity percentages
     severity_distribution = compute_severity_percentages(severity)