feat(skills): add a system-modeling skill by zhangfand · Pull Request #292 · rome-os/rome

zhangfand · 2026-09-08T15:45:51Z

What this PR does

Designing a system in conversation leaves the model in the transcript. The next session rebuilds it from scratch and rediscovers the same defects: a verb reified into a noun, a message mistaken for a state, one component reading another component's memory instead of the shared store, a state with no way back.

This adds a skill that makes the process repeatable and produces three artifacts a reader can review without the conversation.

Design & Invariants

Three passes, one file each, ordered so that every pass can falsify the one before it.

Vocabulary. Five or fewer nouns, their verbs, and numbered rules. Every term is defined by the property that must hold, never by how the current code achieves it.
Pseudocode. Nouns become classes, verbs become methods, rules become asserts. The deterministic part is one pure function from a store snapshot to a list of actions, and every place a model is called is named. Whatever the code needs that no verb names goes back into the vocabulary as an open decision.
Diagrams. One sequence diagram per scenario, plus a state machine split into ownership states and derived positions. Each diagram is read for a specific class of defect.

The invariant that holds the three together: the vocabulary is the model, and the other two artifacts are instruments. A defect a diagram exposes is fixed in the vocabulary first and flows back down, so the files never disagree about what the system is.

Two decisions worth naming:

Separate from domain-modeling rather than folded into it. That skill maintains a glossary in CONTEXT.md as an ongoing habit. This one designs a model from nothing and hands the glossary over when it is signed off. Merging them would have put a five-step design process behind a skill whose job is a one-line reading habit.
The scenario checklist is fixed, not suggested. Ten scenarios, including a process death between two writes, a restart that leaves a worker unreachable, and a person doing the worker's job by hand. Each one broke a revision of the model the skill was extracted from, which is what earns it a place in the list.

render/ renders the markdown to the monochrome style the artifacts were reviewed in, so the user reads diagrams rather than Mermaid source.

Test plan

scripts/lint-shell.sh passes on render/render.sh (shfmt and shellcheck), after pnpm format:sh
scripts/check-pr-title.sh accepts the title
render/render.sh rendered two real design documents end to end: 15 Mermaid diagrams present, the noun/verb/rule count row computed from the vocabulary tables, footer applied
Confirmed the prose gate does not reach here — scripts/check-prose.mjs scopes Vale to docs/*.md and tracked .ts/.tsx
Not yet driven from step 1 on a fresh design; the skill is written from one worked case

Not in this PR

The design documents the skill was extracted from. They belong with the system they describe, not with the method.
Any change to domain-modeling. The handoff is named in SKILL.md and needs nothing on that side.
A worked example under the skill. Adding one costs context load on every run, and the formats already carry the shapes.

🤖 Generated with Claude Code

Designing a system in chat leaves the model in the transcript. The next session rebuilds it from scratch and rediscovers the same defects: a verb reified into a noun, a message mistaken for a state, a component reading another component's memory instead of the shared store. The skill makes that a three-pass process with one reviewable artifact per pass. The vocabulary is the model; the pseudocode and the diagrams are instruments that falsify it. A defect found downstream is fixed in the vocabulary first and flows back down. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Jessie-QingYu

Code Review: 🛑 REQUEST_CHANGES

This PR adds a project-level system-modeling workflow with vocabulary, pseudocode, scenario, and diagram guidance, plus a Pandoc/Mermaid renderer. I read all eight new files, reviewed the repository's skill and documentation conventions, and traced the renderer entry points; there are no automated tests for the Python postprocessor or browser-side rendering.

The overall three-pass direction is coherent, but the supplied pseudocode performs a model call inside the function described as pure and restart-safe. The renderer also inserts error text as HTML, and several secondary integration and correctness issues remain.

Verdict: REQUEST_CHANGES — The skill's reference model violates its core determinism invariant, and the renderer contains an HTML-injection path that should be fixed before merge.

5 finding(s) posted as inline comments below.

Severity	Category	File	Title
P1	architecture	`.claude/skills/system-modeling/PSEUDOCODE.md`	Keep the model call outside the pure reconciler
P1	security	`.claude/skills/system-modeling/render/header.html`	Render Mermaid failures as text rather than HTML
P2	architecture	`.claude/skills/system-modeling/SKILL.md`	Do not require an unavailable handoff skill
P2	error-handling	`.claude/skills/system-modeling/render/postprocess.py`	Fix the incomplete stats removal
P2	design	`.claude/skills/system-modeling/SKILL.md`	Do not store invalid pseudocode as a Python source file

Automated review by RomeOS Code Review · commit 5f81089

Jessie-QingYu · 2026-09-08T15:58:02Z

+            Created:                 Take(task), Start(task)
+            Taken:                   Start(task)
+            Started:                 nothing                     # working
+            Returned(_, reply):      Report(task) if judge(task, reply) else Start(task, reason)


[P1] architecture — Keep the model call outside the pure reconciler

reconcile is described as deterministic and pure, but this branch calls judge, which line 67 explicitly identifies as a model call. Besides being nondeterministic, a process death after the call but before the action write causes the next pass to repeat the model call. Return a Judge action, persist its result as a fact, and let the next reconciliation produce Report or Start.

Jessie-QingYu · 2026-09-08T15:58:02Z

+      pre.innerHTML = svg;
+      pre.dataset.processed = "true";
+    } catch (e) {
+      pre.insertAdjacentHTML("afterbegin", "<p class=\"diagram-error\">diagram failed: " + String(e.message || e) + "</p>");


[P1] security — Render Mermaid failures as text rather than HTML

Mermaid parser errors can contain the offending diagram source, but this code concatenates the error into insertAdjacentHTML. A malformed diagram containing an event-handler tag can therefore execute script when the generated page opens. Create the <p> node and assign textContent; apply equivalent escaping to the footer's --source value in postprocess.py.

Jessie-QingYu · 2026-09-08T15:58:02Z

+## 5. Propagate and hand off
+
+- A decision lands in the vocabulary first, then the pseudocode, then the scenarios, in that order, with a change-log entry in each.
+- When the model is signed off, hand the glossary to the `domain-modeling` skill for `CONTEXT.md`, and the deterministic function and the scenarios to whoever builds the prototype: the scenarios are its test cases.


[P2] architecture — Do not require an unavailable handoff skill

Neither a domain-modeling skill nor a CONTEXT.md contract exists in this repository, so a clean checkout cannot complete this required final step. Rome's existing terminology owner is docs/concepts/. Include the dependency, make the handoff optional and self-contained, or target the repository's existing concepts workflow.

Jessie-QingYu · 2026-09-08T15:58:02Z

+
+
+def insert_stats(html: str) -> str:
+    html = re.sub(r'<div class="stats">.*?</div>\s*', "", html, flags=re.S)


[P2] error-handling — Fix the incomplete stats removal

This non-greedy pattern stops at the first nested </div> inside .stats, leaving the remaining stat nodes behind. Running the postprocessor twice produces duplicate cards and unmatched closing tags, contrary to its documented idempotence. Remove the complete wrapper with an HTML parser or surround the generated block with unambiguous markers.

Jessie-QingYu · 2026-09-08T15:58:02Z

+
+## 3. Pseudocode
+
+Produce `<topic>.py` in the conventions in [PSEUDOCODE.md](PSEUDOCODE.md).


[P2] design — Do not store invalid pseudocode as a Python source file

The referenced format deliberately uses non-Python grammar such as enum Trigger and Type?, yet this instruction gives it a .py extension. Host repositories using Ruff, mypy, compileall, or generic Python discovery can consequently fail after the skill creates its artifact. Use a fenced <topic>-pseudocode.md or another non-source extension, or require valid Python syntax.

Jessie-QingYu reviewed Sep 8, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(skills): add a system-modeling skill - #292

feat(skills): add a system-modeling skill#292
zhangfand wants to merge 1 commit into
mainfrom
skills/system-modeling

zhangfand commented Sep 8, 2026

Uh oh!

Jessie-QingYu left a comment

Uh oh!

Jessie-QingYu Sep 8, 2026

Uh oh!

Jessie-QingYu Sep 8, 2026

Uh oh!

Jessie-QingYu Sep 8, 2026

Uh oh!

Jessie-QingYu Sep 8, 2026

Uh oh!

Jessie-QingYu Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		def insert_stats(html: str) -> str:
		html = re.sub(r'<div class="stats">.?</div>\s', "", html, flags=re.S)


		## 3. Pseudocode

		Produce `<topic>.py` in the conventions in [PSEUDOCODE.md](PSEUDOCODE.md).

Uh oh!

Conversation

zhangfand commented Sep 8, 2026

What this PR does

Design & Invariants

Test plan

Not in this PR

Uh oh!

Jessie-QingYu left a comment

Choose a reason for hiding this comment

Code Review: 🛑 REQUEST_CHANGES

Uh oh!

Jessie-QingYu Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Jessie-QingYu Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Jessie-QingYu Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Jessie-QingYu Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Jessie-QingYu Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants