From d86adaf7cc9eea0af6552cb1cd75c1a551b31fcc Mon Sep 17 00:00:00 2001 From: root Date: Sat, 25 Apr 2026 22:54:03 -0300 Subject: [PATCH] Diagram procedure: forbid workload-driven shape, warn at runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Codex session showed why the workload-driven mode is the wrong default. Codex authored a spec under ``tenancy → regions → compartments → services`` (the documented "workload-driven" mode), generated two .drawio files, and reported success. The output was unusable: services rendered as colored rectangles with text instead of OCI icon stencils, connector text went into the edge value (forbidden by the connector-label rule), and the spec validator never ran because it only fires on ``absolute_layout`` blocks. Diego: "los drawio son horribles, por que sucede eso?". Three persistent fixes: 1. SKILL.md option 2 now says ``absolute_layout`` is REQUIRED for step 3, calls out workload-driven explicitly as forbidden, and explains *why* (no stencils, no validator). 2. docs/skill/output-formats.md § Diagram modes is rewritten — mode 1 is ``absolute_layout`` (REQUIRED), mode 2 is workload-driven (DEPRECATED, back-compat only). Removes the "two acceptable shapes" framing that let Codex pick the wrong one. 3. ``oci_diagram_gen.py`` now prints a loud stderr WARNING when a spec takes the workload-driven path, naming the procedure docs. Catches any agent that skipped reading SKILL.md before generating. Codex copy synced via ``make sync-skill``. Co-Authored-By: Claude Opus 4.7 (1M context) --- .agents/skills/oci-deal-accelerator/SKILL.md | 4 ++-- SKILL.md | 4 ++-- docs/skill/output-formats.md | 6 ++--- tools/oci_diagram_gen.py | 24 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/.agents/skills/oci-deal-accelerator/SKILL.md b/.agents/skills/oci-deal-accelerator/SKILL.md index 9862f71..05420c3 100644 --- a/.agents/skills/oci-deal-accelerator/SKILL.md +++ b/.agents/skills/oci-deal-accelerator/SKILL.md @@ -135,8 +135,8 @@ Pick a number, or just describe what you need. After the user answers, **follow these steps in order — do NOT skip step 1**: 1. **Reference-architecture lookup.** Run `python tools/archcenter_pattern_lookup.py ""` against `kb/architecture-center/catalog.yaml` (123 Oracle-curated entries with cached `.drawio` / `_description.md` under `kb/diagram/assets/archcenter-refs/`). Pick the highest-scoring entry whose topology matches; copy its container nesting, padding, and AD/subnet placement. **Do NOT search `examples/` for references** — `examples/` are previous user outputs, not authoritative Oracle conventions. 2. **Pre-generation review.** Confirm the component list with the user (REQUESTED + TECHNICAL DEPENDENCIES per the whitelist). - 3. **Author the `absolute_layout` spec** following the geometry rules. - 4. **Spec validator runs automatically** before either renderer (`tools/diagram_spec_validator.py`). Fix any errors. + 3. **Author the spec in `absolute_layout` shape** — this is REQUIRED. Do NOT use the legacy workload-driven shape (`tenancy → regions → compartments → services`); it does not resolve OCI icon stencils, does not run through the spec validator, and produces wireframe-looking output (rectangles with text instead of real OCI icons). Every container, service, label, and connection needs explicit `(x, y, w, h)`. + 4. **Spec validator runs automatically** before either renderer (`tools/diagram_spec_validator.py`). It runs ONLY on `absolute_layout` specs — that's another reason workload-driven is forbidden. Fix any errors before re-rendering. 5. **Render.** `oci_diagram_gen.py` for drawio, `oci_deck_gen.py` for PPTX. 6. **Visually verify.** `tools/oci_pptx_render.py` to rasterize, then read the PNG. diff --git a/SKILL.md b/SKILL.md index 1aa89e8..0b9548f 100644 --- a/SKILL.md +++ b/SKILL.md @@ -133,8 +133,8 @@ Pick a number, or just describe what you need. After the user answers, **follow these steps in order — do NOT skip step 1**: 1. **Reference-architecture lookup.** Run `python tools/archcenter_pattern_lookup.py ""` against `kb/architecture-center/catalog.yaml` (123 Oracle-curated entries with cached `.drawio` / `_description.md` under `kb/diagram/assets/archcenter-refs/`). Pick the highest-scoring entry whose topology matches; copy its container nesting, padding, and AD/subnet placement. **Do NOT search `examples/` for references** — `examples/` are previous user outputs, not authoritative Oracle conventions. 2. **Pre-generation review.** Confirm the component list with the user (REQUESTED + TECHNICAL DEPENDENCIES per the whitelist). - 3. **Author the `absolute_layout` spec** following the geometry rules. - 4. **Spec validator runs automatically** before either renderer (`tools/diagram_spec_validator.py`). Fix any errors. + 3. **Author the spec in `absolute_layout` shape** — this is REQUIRED. Do NOT use the legacy workload-driven shape (`tenancy → regions → compartments → services`); it does not resolve OCI icon stencils, does not run through the spec validator, and produces wireframe-looking output (rectangles with text instead of real OCI icons). Every container, service, label, and connection needs explicit `(x, y, w, h)`. + 4. **Spec validator runs automatically** before either renderer (`tools/diagram_spec_validator.py`). It runs ONLY on `absolute_layout` specs — that's another reason workload-driven is forbidden. Fix any errors before re-rendering. 5. **Render.** `oci_diagram_gen.py` for drawio, `oci_deck_gen.py` for PPTX. 6. **Visually verify.** `tools/oci_pptx_render.py` to rasterize, then read the PNG. diff --git a/docs/skill/output-formats.md b/docs/skill/output-formats.md index 04212da..1f9f7c4 100644 --- a/docs/skill/output-formats.md +++ b/docs/skill/output-formats.md @@ -85,10 +85,10 @@ These rules are enforced by `tools/diagram_spec_validator.py` and exist because ### Diagram modes -The diagram generator accepts two spec shapes: +The diagram generator accepts two spec shapes — but only ONE is acceptable for any output an SA shows a customer: -1. **Workload-driven (default)** — `tenancy → region(s) → vcn(s) → subnet(s) → service(s)` plus on-premises and external actors. The generator auto-lays out containers and service blocks. Use this for most customer proposals where you describe the workload abstractly. -2. **`absolute_layout` (fidelity mode)** — explicit `(x, y, w, h)` for every container, service, label, and connection. Use this when the user wants to reproduce an Oracle Architecture Center reference diagram with maximum visual fidelity, or when a layout already exists (e.g. extracted from an official `.drawio`). +1. **`absolute_layout` (REQUIRED for any new diagram).** Explicit `(x, y, w, h)` for every container, service, label, and connection. Resolves real OCI stencils from `kb/diagram/oci-icons.json`, runs through `tools/diagram_spec_validator.py` (geometry rules), and produces output visually consistent with Oracle Architecture Center references. **This is the only mode the standard procedure supports.** Pair it with the ref-arch lookup (`tools/archcenter_pattern_lookup.py`) so geometry derives from a canonical Oracle reference, not invented from scratch. +2. **Workload-driven (DEPRECATED).** `tenancy → region(s) → vcn(s) → subnet(s) → service(s)`. The generator auto-lays out blocks but **does NOT resolve OCI icon stencils** — services render as colored rectangles with text labels, connector text gets baked into edge values (which the connector-label rule forbids), and the spec validator does NOT run. Output looks like a wireframe placeholder, not an architecture diagram. Kept only for back-compat with very old specs; do not author new specs in this shape. ```yaml absolute_layout: diff --git a/tools/oci_diagram_gen.py b/tools/oci_diagram_gen.py index a5fb519..893511f 100644 --- a/tools/oci_diagram_gen.py +++ b/tools/oci_diagram_gen.py @@ -2198,6 +2198,30 @@ class OCIDiagramGenerator: gen._render_absolute_layout(spec) return gen + # Legacy workload-driven shape (tenancy → regions → compartments + # → services). Does NOT resolve OCI stencils — services render + # as colored rectangles with text labels — does NOT run through + # the spec validator, and bakes connector text into edge values + # (forbidden by the connector-labels rule). Output looks like a + # wireframe placeholder, not an architecture diagram. Loud + # warning so any agent that takes this path against the + # procedure docs sees the problem immediately, even if it + # skipped reading SKILL.md option 2. + import sys as _sys + if spec.get("tenancy") or spec.get("regions") or spec.get("clouds"): + print( + "[oci_diagram_gen] WARNING: spec uses the legacy " + "workload-driven shape (tenancy → regions → " + "compartments → services). This mode does NOT resolve " + "OCI icon stencils, does NOT run the spec validator, " + "and produces wireframe-looking output. The standard " + "procedure (SKILL.md option 2 / docs/skill/output-formats.md) " + "requires the `absolute_layout` shape. Re-author the " + "spec under `absolute_layout:` for any customer-facing " + "diagram.", + file=_sys.stderr, + ) + # External actors (users, internet, third-party) for ext in spec.get("external", []): ext_id = ext.get("id", gen._next_id())