feat: add Mermaid graph-coherence linter and graph shape rules by jnyross · Pull Request #9 · jnyross/codex-graph

jnyross · 2026-08-07T22:43:48Z

Summary

Adds an objective, stdlib-only Mermaid graph-coherence linter and three design rules so a generated workflow is executable as drawn, and wires it into the bundle so it is enforced automatically.

New skills/codex-graph/scripts/graph_coherence.py — parses flowchart TD/LR blocks and flags orphaned nodes, dead-end/stranded nodes (non-terminal that cannot reach a terminal), and nodes unreachable from any start. Comes with a --selfcheck unit test covering all four cases.
SKILL.md — new design rules 15–17 plus matching quality-check bullets:
1. graphs must be complete and reachable (no orphan/stranded/unreachable nodes);
2. many files ≠ parallel — a tightly-coupled, dependency-ordered change (e.g. a repo-wide rename of one symbol) stays an L0/L1 small change rather than a fan-out;
3. Rationale↔graph coherence — the Rationale must describe the graph actually drawn (same task family, baseline tier, parallel/serialized split); fix the graph if they disagree.
validate_skill.py — enforces the new rules and lints the bundle’s own Mermaid diagrams (SKILL.md + topology-library.md) for coherence.
run_windows_regressions.ps1 — runs the coherence linter selfcheck in CI.
README — documents the linter.

Validation

Added a regression case for new behavior: graph_coherence.py --selfcheck (wired into run_windows_regressions.ps1).
Ran .\scripts\run_windows_regressions.ps1 from Codex Desktop on Windows.
If no regression case is appropriate, explained why below.

I ran every step of the suite locally on macOS (not the literal Windows .ps1):

python3 skills/codex-graph/scripts/validate_skill.py → PASS
python3 skills/codex-graph/scripts/graph_coherence.py --selfcheck → PASS
node --test skills/codex-graph/scripts/task_collection_harness.test.js → 6 pass, 0 fail
python3 scripts/test_auto_release.py → 4 tests OK

The PR CI job runs the full Windows regression suite on windows-latest.

Test notes

New regression case: graph_coherence.py --selfcheck covers a coherent diagram plus three incoherent ones (orphan node, dead-end cycle, unreachable cycle). validate_skill.py also now fails any skill-bundle Mermaid with an incoherent diagram.

Note

Low Risk
Changes are limited to skill docs, a new validation script, and CI wiring; no runtime product or auth/data paths.

Overview
Adds a stdlib-only Mermaid linter (graph_coherence.py) that parses flowchart TD/LR diagrams and fails on orphans, dead-end/stranded nodes, unreachable islands, and edges to undefined nodes, with a --selfcheck harness for positive and negative cases.

SKILL.md gains design rules 15–17 and matching quality checks: graphs must be complete and reachable (run the linter or mirror its rules), many files ≠ parallel (don’t fan out tightly coupled work), and Rationale must match the graph (same tier and parallel/serialize split).

validate_skill.py now requires those phrases in the skill text and runs coherence checks on Mermaid in SKILL.md and topology-library.md. run_windows_regressions.ps1 runs graph_coherence.py --selfcheck; README documents the linter.

^{Reviewed by Cursor Bugbot for commit fdcc8dd. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

New Features
- Added Mermaid graph coherence validation to identify disconnected, undefined, orphaned, dead-end, and unreachable workflow nodes.
- Added support for validating multiple diagrams with combined diagnostics and self-checks.
Bug Fixes
- Windows regression checks now include graph coherence validation and report failures correctly.
Documentation
- Updated workflow guidance and validation documentation to require complete, reachable graphs and appropriate parallelization rationale.

Adds an objective, stdlib-only Mermaid flowchart linter (skills/codex-graph/scripts/graph_coherence.py) that flags orphaned, stranded/dead-end, and unreachable graph nodes so a workflow is executable as drawn, and wires it into the skill bundle: - SKILL.md: new design rules 15-17 (complete/reachable graphs, many-files- not-parallel for tightly-coupled repo-wide changes, Rationale must match the graph actually drawn) plus matching quality-check bullets. - validate_skill.py: enforces the new rules and lints the bundle's own Mermaid diagrams for coherence. - run_windows_regressions.ps1: runs the coherence linter selfcheck in CI. - README: documents the linter. All regression-suite steps pass locally (validate_skill, coherence selfcheck, node harness, auto-release tests).

coderabbitai · 2026-08-07T22:44:04Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The change adds a Mermaid flowchart coherence linter. Skill validation and Windows regressions now run it. Graph guidance and README instructions define the new checks.

Changes

Graph coherence validation

Layer / File(s)	Summary
Graph coherence linter `skills/codex-graph/scripts/graph_coherence.py`	The new module parses Mermaid flowcharts, detects structural errors, provides self-tests, and supports file and CLI linting.
Validation and regression integration `skills/codex-graph/scripts/validate_skill.py`, `scripts/run_windows_regressions.ps1`, `README.md`	Skill validation checks required graph guidance and Mermaid coherence. The Windows script runs the linter self-check and propagates failures. README instructions document the linter.
Workflow graph requirements `skills/codex-graph/SKILL.md`	Workflow requirements and the quality checklist now cover reachability, terminal paths, parallelization constraints, graph validation, and rationale consistency.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Windows regression script
  participant validate_skill.py
  participant graph_coherence.py
  participant Mermaid documents
  Windows regression script->>graph_coherence.py: Run --selfcheck
  validate_skill.py->>graph_coherence.py: Check Mermaid blocks
  graph_coherence.py->>Mermaid documents: Parse flowchart nodes and edges
  graph_coherence.py-->>validate_skill.py: Return coherence diagnostics
  graph_coherence.py-->>Windows regression script: Return exit status

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: adding a Mermaid graph-coherence linter and graph design rules.
Description check	✅ Passed	The description covers the behavior changes, validation results, regression case, test notes, and the reason Windows execution was not performed locally.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/graph-coherence-and-shape-rules

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

devin-ai-integration · 2026-08-08T08:06:13Z

Fixed all Devin Review findings in fa14118 — the regex-scanning parser is replaced by a statement scanner:

Chained links / non---> forms: A --> B --> T, ==>, -.->, --x/--o, long arrows, -- text --> / -. text .->, |label|, A & B --> C, and ;-separated statements all parse now (selfcheck fixtures chained, link_variants, fan_out, tight_spacing).
Flow directions: block regex accepts flowchart/graph with TD|TB|BT|LR|RL (fixture orphan_other_direction proves a TB orphan is caught).
Quoted labels: shapes are scanned with a quote- and depth-aware scanner, so A["Start (init)"] and A[[Sub]]/((Circle))/{{Hex}} are read whole (fixture quoted_label).
Subgraphs: subgraph/end/direction/classDef/class/style/linkStyle/click lines no longer declare nodes; a subgraph id becomes a node only when it is an edge endpoint (fixture subgraph).
No-arg CLI: lints a diagram piped on stdin, otherwise prints usage and exits 2 instead of a false pass. SKILL.md rule 15 now shows a runnable form and the quality-check bullet uses the real bundle path.

Undefined-node detection is preserved: only a shape declaration defines a node, so A[Start] --> B and A[Start] --> B[Work] --> T still report edge references undefined node.

Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

devin-ai-integration

Devin Review found 2 new potential issues.

View 1 additional finding in Devin Review.

devin-ai-integration · 2026-08-08T08:16:31Z

+            index = match.end()
+            label: Optional[str] = None
+            if index < length and stmt[index] in _SHAPE_OPENERS:
+                scanned = _scan_shape(stmt, index)
+                if scanned is None:


🟡 Diagrams that loop a step back onto itself are passed as valid

A step that points back to itself is thrown away instead of recorded (if a != b at skills/codex-graph/scripts/graph_coherence.py:174), so a workflow whose last step spins on itself forever is reported as a valid ending.
Impact: The linter gives a clean bill of health to a diagram containing an endless self-loop, exactly the kind of non-terminating graph the checks are meant to catch.

Self-edges are filtered out before graph construction, turning a looping node into an apparent terminal

In _parse_statement, edges are only appended when a != b, so B --> B produces no edge at all. Verified: check_text('flowchart TD\n A[S] --> B[W]\n B --> B\n') returns []. Because B then has out-degree 0, Graph.terminals() (skills/codex-graph/scripts/graph_coherence.py:85-86) classifies it as a terminal, and the dead-end scan at skills/codex-graph/scripts/graph_coherence.py:237-254 is satisfied. The multi-node loop case (L --> L2 --> L) is covered by the dead_end selftest, but the single-node loop — the most common shape of an unbounded retry (V1 -->|retry| V1) — is not. A lone self-looping node is also mis-reported as "orphaned" rather than a dead end. The a != b guard is only needed to avoid duplicates inside & groups; self-links should be kept.

Prompt for agents

In skills/codex-graph/scripts/graph_coherence.py, _parse_statement drops any edge whose source equals its target (the `if a != b` filter when expanding previous x group). This means a Mermaid self-loop such as `V1 --> V1` produces no edge, so the node keeps out-degree 0 and check_graph treats it as a legitimate terminal — an unbounded self-loop passes the coherence check, and a node whose only edge is a self-loop is reported as 'orphaned' instead of a dead end. The `a != b` guard is only needed to suppress duplicate pairs generated when the same id appears in both sides of an `&` group; it should not suppress an explicitly written self-link. Consider recording self-edges (deduplicating only within the `&` expansion) and adding a selftest case for a single-node self-loop that must be reported as unable to reach a terminal.

Was this helpful? React with 👍 or 👎 to provide feedback.

Fixed in 357eefd. Self-links are recorded now; the dedupe moved to a per-link emitted set so it still suppresses duplicate pairs from an & expansion. V1 -->|retry| V1 is reported as cannot reach any terminal (selfcheck fixture self_loop), and a lone self-looping node also reports unreachable from any start node rather than being called an orphan.

devin-ai-integration · 2026-08-08T08:16:32Z

+            index = close + 1
+
+
+def parse_flowchart(body: str) -> Graph:
+    nodes: List[str] = []
+    labels: Dict[str, str] = {}
+    edges: List[Tuple[str, str]] = []
+    subgraph_ids: Set[str] = set()
+    statements = [
+        stmt
+        for raw_line in body.splitlines()


🟡 Valid diagrams that connect a group of steps as a block are wrongly rejected

Steps listed inside a group are treated as having no connections (orphan check at skills/codex-graph/scripts/graph_coherence.py:233-235) when the diagram wires the group as a whole rather than each step, so a correct diagram is reported as broken.
Impact: A legitimate workflow diagram fails the linter, and adding such a diagram to the bundle would break the validator that now runs it.

Subgraph membership is not modelled, so nodes reachable only via the container id look edge-less

parse_flowchart records a subgraph id (skills/codex-graph/scripts/graph_coherence.py:206-216) but never links its member nodes to it. Mermaid permits N1 --> P / P --> N3 where P is a subgraph containing N2A, N2B. Verified: check_text('flowchart TD\n subgraph P[Par]\n N2A[a]\n N2B[b]\n end\n N1[S] --> P\n P --> N3[J]\n') returns ['diagram #1 (TD): orphaned nodes with no edges: N2A, N2B']. The same members would also be reported as unreachable/dead-end if they had only internal edges among themselves. Since validate_skill.py:298-306 now hard-fails on any reported violation, a future bundle diagram in this style would break validation.

Prompt for agents

skills/codex-graph/scripts/graph_coherence.py collects subgraph ids in parse_flowchart but does not associate member nodes with their container. When a diagram wires the container id as an edge endpoint (a legal Mermaid pattern: `N1 --> P`, `P --> N3`, with `N2A`/`N2B` declared inside `subgraph P ... end`), the member nodes have no edges of their own and check_graph reports them as orphaned / unreachable / dead-end. Consider tracking subgraph membership while parsing (track the current container while between `subgraph` and `end`) and, when the container id is used as an edge endpoint, treating edges into/out of the container as edges into/out of its members (or otherwise exempting members of an edge-connected container from the orphan/reachability checks). Add a selftest for this pattern.

Was this helpful? React with 👍 or 👎 to provide feedback.

Fixed in 357eefd. parse_flowchart now tracks an open-container stack (nested subgraphs included) and records membership, then _expand_container_edges wires container-level links through to the members they stand for: an edge into P also enters every member with no internal in-edge, and an edge out of P also leaves every member with no internal out-edge.

Your case now passes, and subgraph P[Par]\n X1[a] --> X2[b]\n end\n N1 --> P\n P --> N3 correctly yields N1 --> X1 and X2 --> N3 only (entry/exit members, not every member). Selfcheck fixture container_edges added.

Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

coderabbitai

🧹 Nitpick comments (1)

skills/codex-graph/scripts/graph_coherence.py (1)

352-357: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the parsed directions in the self-test.

This fixture passes if every operator regresses to forward direction. That regression produces A -> B, T -> B, and B -> C, which is still coherent under check_text.

Compare parse_flowchart(_SELFTEST["link_directions"]).edges with the expected set. Include ("B", "T") for <-- and both ("B", "C") and ("C", "B") for <-->.

Proposed test addition

 def selftest() -> int:
     fails = 0
+    expected_direction_edges = {
+        ("A", "B"),
+        ("B", "T"),
+        ("B", "C"),
+        ("C", "B"),
+    }
+    actual_direction_edges = set(
+        parse_flowchart(_SELFTEST["link_directions"]).edges
+    )
+    if actual_direction_edges != expected_direction_edges:
+        print(
+            "  [FAIL] link directions parsed incorrectly:",
+            actual_direction_edges,
+        )
+        fails += 1
 
     for name in [

Also applies to: 432-432

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/codex-graph/scripts/graph_coherence.py` around lines 352 - 357, Update
the self-test for the "link_directions" fixture to parse it with parse_flowchart
and assert its edges against the expected set. Ensure the expected directions
include ("A", "B"), ("B", "T") for "<--", and both ("B", "C") and ("C", "B") for
"<-->" rather than relying only on check_text coherence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@skills/codex-graph/scripts/graph_coherence.py`:
- Around line 352-357: Update the self-test for the "link_directions" fixture to
parse it with parse_flowchart and assert its edges against the expected set.
Ensure the expected directions include ("A", "B"), ("B", "T") for "<--", and
both ("B", "C") and ("C", "B") for "<-->" rather than relying only on check_text
coherence.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad1084cd-a727-4c51-8e3a-7ebdc7b80902

📥 Commits

Reviewing files that changed from the base of the PR and between ffc8f17 and e50e3ca.

📒 Files selected for processing (2)

skills/codex-graph/scripts/graph_coherence.py
skills/codex-graph/scripts/validate_skill.py

🚧 Files skipped from review as they are similar to previous changes (1)

skills/codex-graph/scripts/validate_skill.py

…mbers Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

…lently Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>

Co-Authored-By: jnyross@gmail.com <jnyross@gmail.com>