From 15c45d3b6b1d7a61c8d51069d6cd77a2314ba6dc Mon Sep 17 00:00:00 2001 From: Bai Ming Date: Mon, 5 Oct 2026 22:46:49 -0700 Subject: [PATCH 1/2] Move the design-tree skill here from Whitefoot SKILL.md, lint.py and test_lint.py come from Whitefoot cf6b84ff3, identical to Snowghost's copy. The CI base script moves in from .github/design-review-base.sh as review-base.sh, since nine of test_lint.py's cases run it, and test_lint.py finds both scripts beside itself instead of at a project's paths. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/test.yml | 14 ++ README.md | 148 +++++++++++++++++- design-tree/SKILL.md | 265 ++++++++++++++++++++++++++++++++ design-tree/lint.py | 299 +++++++++++++++++++++++++++++++++++++ design-tree/review-base.sh | 42 ++++++ design-tree/test_lint.py | 222 +++++++++++++++++++++++++++ 6 files changed, 989 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/test.yml create mode 100644 design-tree/SKILL.md create mode 100644 design-tree/lint.py create mode 100644 design-tree/review-base.sh create mode 100644 design-tree/test_lint.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..cff35a9 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,14 @@ +name: test + +on: + push: + pull_request: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - name: Test the lint and the CI base selection + run: python3 -B -m unittest discover -s design-tree -p 'test_lint.py' -v diff --git a/README.md b/README.md index 3eaa36d..f2626f5 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,149 @@ # Design-skill -The design-tree skill, kept in one place for every project that uses it. +A skill for coding agents that keeps a project's design decisions in a +**design tree**: each decision written down with the reason it was made and +the alternatives it beat, approved by the project's owner before it reaches +the main line, and checked against the code that implements it. + +It works with Claude Code and Codex, and it is written for any project. Its +checks are a small Python script and a test suite that run locally and in +CI. + +## Why + +Agents write a lot of code quickly, and the reasons behind it are easy to +lose: they end up spread over pull request threads and conversations, or are +never written down at all. Later work then proposes again an alternative that +was already rejected, or quietly contradicts a choice nobody can find. A +design tree keeps the reasons in the repository, next to the code, in a form +short enough to read and strict enough to check: + +- **One place for decisions.** The tree is organized by concept, not by code + structure. Each node holds one or more decisions and the alternatives they + refused. +- **The owner decides.** An agent may change the tree freely on a draft + branch, but the branch cannot become ready until the owner has approved + every change and the approval is recorded in the log. CI enforces this. +- **Code follows the decisions.** At completion, a separate reviewer checks + the tree's changes and checks the code against the decisions it implements + (the Design Correspondence Review). + +## What a tree looks like + +A project keeps its tree in a directory, usually `design/`: one Markdown file +per node, children in a directory named after their parent, and a change log +beside the root nodes. + +``` +design/ + log.md one entry per approved change, newest first + pipeline.md a root node + pipeline/ + style.md a child of pipeline + layout.md +``` + +A node holds `Decision:` lines, each stating the choice, its reason after +`because` and the alternative after `instead of`, and an optional `Rejected:` +list: + +``` +Decision: Layout lengths are 1/64-pixel fixed-point integers, because fixed +point adds and compares exactly and keeps layout independent of +floating-point rounding, instead of f32 or f64 lengths. + +Rejected: +- f64 lengths: rejected because a layout would depend on evaluation order. +``` + +Each field is one line in the file; the example wraps it for reading. A log +entry names the nodes it changed and records the owner's approval: + +``` +## 2026-10-01 Use fixed-point layout lengths + +Nodes: vocabulary + +Owner-approved: The owner agreed to Q12 as recommended. + +Summary: Layout lengths become 1/64-pixel integers; the measurements are in +research/investigations/vocabulary/DESIGN.md. +``` + +[`design-tree/SKILL.md`](design-tree/SKILL.md) is the full procedure: the +node format, what counts as a decision, how the owner's decisions are +gathered into decision cards at handoff, the log format and the review +checks. + +## Contents + +| File | Purpose | +|---|---| +| `design-tree/SKILL.md` | The skill, loaded by the agent when a task matches its description | +| `design-tree/lint.py` | Form check of a tree and, with `--require-approval`, the readiness check | +| `design-tree/review-base.sh` | Picks the base revision a CI job compares the tree with | +| `design-tree/test_lint.py` | Tests for the two scripts above | + +The scripts need Python 3, Git and a POSIX shell. + +## Using it in a project + +A project keeps its own copy of `design-tree/`, which it does not edit: +changes are made here, so every project runs the same skill. + +1. **Copy it** into the project, for example as `design/skill/`, and name + the Design-skill commit it came from in the commit message. + +2. **Expose it to the agents.** From the project's root, link it where Claude + Code and Codex look for skills: + + ```sh + mkdir -p .claude/skills .agents/skills + ln -s ../../design/skill .claude/skills/design-tree + ln -s ../../design/skill .agents/skills/design-tree + ``` + +3. **Map its roles** in the project's agent instructions (`AGENTS.md` or + `CLAUDE.md`): where the live tree and the log live, where research records + and deferred work go, and which commands run the checks. + +4. **Wire the checks**, for example in a Makefile: + + ```make + DESIGN_REVIEW_BASE ?= origin/main + design-lint: + python3 -B -m unittest discover -s design/skill -p 'test_lint.py' + python3 -B design/skill/lint.py --root design --trees pipeline --base "$(DESIGN_REVIEW_BASE)" + design-ready: + python3 -B design/skill/lint.py --root design --trees pipeline --base "$(DESIGN_REVIEW_BASE)" --require-approval + ``` + + Run `design-lint` on every push, and `design-ready` on pull requests that + are ready for review and on the main line. In CI, pick the base with + `review-base.sh`, which needs the full history (`fetch-depth: 0`): + + ```sh + base=$(sh design/skill/review-base.sh "$GITHUB_EVENT_NAME" "$GITHUB_REF" "$PUSH_BEFORE") + make design-ready DESIGN_REVIEW_BASE="$base" + ``` + + where `PUSH_BEFORE` is `${{ github.event.before }}`. A draft branch may + change the tree without an approval; `design-ready` is what keeps + unapproved changes off the main line. + +## Changing the skill + +Change it here, by pull request, with the tests passing. Then copy the merged +revision into each project in that project's own pull request, which names +the Design-skill commit it adopts. + +## Projects using it + +- [Whitefoot](https://github.com/Ming-Research/Whitefoot), an + optimizer-first systems language designed to be written by AI +- [Snowghost-wf](https://github.com/Ming-Research/Snowghost-wf), a renderer + for user interfaces built with web technology, written in Whitefoot + +## License + +MIT; see [LICENSE](LICENSE). diff --git a/design-tree/SKILL.md b/design-tree/SKILL.md new file mode 100644 index 0000000..313495b --- /dev/null +++ b/design-tree/SKILL.md @@ -0,0 +1,265 @@ +--- +name: design-tree +description: Keep a project's design decisions in a design tree, bring every decision that needs the owner to the owner, and review an implementation against the recorded decisions. Use when a task makes or changes a choice between viable alternatives, edits the design tree or its log, asks the owner to decide anything, hands finished work back to the owner, or reviews design and implementation for correspondence (DCR). Not for implementing a recorded decision unchanged or for a routine fix. +--- + +# Design tree + +A design tree records the decisions a project is built on: what was chosen, +because of what, instead of what. It is organized by concept, not by code +structure. The project's main line holds only decisions the owner approved; +a draft branch may change the tree freely, and the owner's approval, recorded +in the log, is what lets that branch become ready. Git holds history; the log +records the approvals. + +The project maps these roles to its own paths: + +- Live tree: one file per node, with children in a directory of the same name. +- Change log: one entry per approved change, newest first. +- Research record: where derivations, measurements and comparisons live. +- Maintained TODO: where deferred work is recorded. +- Form check and readiness check: the `lint.py` invocations below. + +## Node format + +A node is a file named for the decision it owns, holding one or more +`Decision:` lines and an optional `Rejected:` list, without dates, standalone +facts, measurements, or progress. Cite evidence in a reason instead. Name +events by what happened, not by date. Each field occupies one line, with a +blank line between fields; list items directly follow their header. + +A `Decision:` line states the choice, its reason after `because`, and the +alternative after `instead of`. At least one must be present; a line with +neither is a description, not a decision. Write for a reader who has not +seen the source record, expanding compressed terminology. + +`Rejected:` lists refused alternatives as `- : rejected because +`, one per line. Give a discriminating reason, and do not re-propose +an alternative without explaining what changed. + +## What is a decision + +A choice between viable alternatives is a decision, even when the selection +seems obvious. An implementation step with only one viable way needs no +record. Start coarse; the owner tunes the threshold when the tree grows too +fine or too thin. + +A reason states its kind of ground. A deduction names its premises and only +the conclusion they entail; an empirical reason names what was observed and +under which conditions; a provisional choice names its reason, uncertainty +and reopening condition. A constitutional principle or one measurement shows +that a choice fits, not that it is the only possible one. Keep an open +question open: name an assumption used to proceed and how it will be checked, +and never record a proposal or an agent's default as a settled decision. + +## Keeping the tree lean + +Apply three filters to every tree change: + +1. Decision, not description. Remove `Decision:` lines without `because` or + `instead of`. +2. Not derivable from code. Remove nodes that only restate an interface or + implementation. +3. Normalize upward. State a shared rule once at its common ancestor instead + of repeating it in children. + +Keep each decision concise: retain the choice, its decisive reason or refused +alternative (or both), and the qualifications needed to preserve its meaning. +Put detailed derivations, measurements, comparisons and implementation +mechanics in the relevant research record and link directly to that section. +A long `Decision:` line is still a long explanation. The tree must explain the +choice without requiring the reader to open the link; the linked record +supplies the supporting detail. + +## Changes and approval + +On a draft branch, change the live tree directly, in the same work as the +implementation it governs, and keep the two consistent as the work goes. +Writing a decision does not approve it. The owner approves at the end, when +the finished work is handed back (Workflow step 4), and approves only the +decisions shown. + +After the owner has ruled on every decision of the branch that needs a +ruling, write one log entry for the approved change and only then mark the +branch ready. The readiness check fails while the tree differs from the base +without such an entry, so unapproved changes cannot reach the main line. A +change made after approval, other than one the owner directed, is shown and +approved again. When the owner refuses a change, revise or revert it; keep a +refused alternative worth remembering as a `Rejected:` item. Approval of the +tree does not authorize a merge; the project's merge rules decide that. + +## Log format + +Each entry has a `## ` heading, a `Nodes:` line listing every +node added, changed or retired, an `Owner-approved:` line identifying the +owner's approval of the handoff in the owner's words, and a concise `Summary:` +paragraph with the change and its reasons. That approval covers every node +the entry names: those in its cards and those in its other tree edits. Write the entry only +after that approval; the field records it and never requests or infers it. +The newest entry must be new on the branch and name every changed node. Cite +data and evidence at their source in the research record instead of +reproducing them. When parallel branches add entries, keep both, newest +first. + +## Owner decisions + +The owner decides in the conversation, in the owner's language. A decision +the owner makes lands in the tree, as a node added, changed or retired or as +a refused option under `Rejected:`, so every decision awaiting the owner is +a tree change, and all of them form one ledger kept in the conversation. +An entry may come from a choice made while working, a review finding, an open +research question or a direction the owner gave in passing. The scope of the +work is agreed before starting (below); a change to it is reported in the +handoff's status, not as an entry. + +- **Entries.** Each gets an ID, `Q1`, `Q2` and on, never renumbered or reused. + It stays open until the owner answers that ID. A discussion that moves past + an entry leaves it open; superseding or withdrawing one needs the owner's + agreement too. +- **Restate.** After every owner reply, list every ID with its status, for + example "Q1, Q2 approved; Q3 approved with a change; Q4, Q5 not yet + discussed". When the owner states a direction in passing, say which entry + it became and whether it is taken as a ruling. +- **Before starting.** Discuss every choice that sets the direction of the + work. While a matter that could change it substantially is unclear, keep + discussing; do not start. +- **After starting.** Work through to completion. A question that arises is + sent to the owner with a recommendation and work continues on that + recommendation; the entry stays open and returns at handoff. +- **Batch.** Bring every open entry to the owner once, at handoff: review + findings go only there, and a question sent while working returns there. + Never bring rulings one round at a time. +- Re-read this skill before a handoff; a copy loaded early in a long session + may predate a change to it. + +A handoff presents, in this order: + +1. **Status.** For each thing the owner asked for: done, done on a + recommendation still open (name the ID), changed from the agreed scope + (how and why), or not started. Research that recommends work is not that + work. +2. **Decision cards.** One per open decision the branch adds, changes or + retires, oldest first, each after the cards it depends on; a card is that + decision's tree change. A decision the owner already ruled on needs no + card; the restated ledger shows it approved. End with one line naming + every open ID and stating that no other decision is open. A card opens + with its ID and the question in bold, then three parts: + - Problem: the problem itself, for a reader who has not seen the work: + what the component or rule does, what goes wrong or stays open, and the + concrete evidence. Explain each project term at first use. + - Options: A, B and on, the recommended one marked. Each says what it + does and what it costs, then why it is recommended or why not. + - Confidence N/5: 5 when evidence settles it, 1 when it rests on judgment, + with the reason and what could overturn it. + + The tree records the ruling: the chosen option becomes the node's + `Decision:` and each refused option worth remembering a `Rejected:` item, + with the reasons the card gave. +3. **Other tree edits.** A node edit that changes no decision, such as a + rewording, needs no card but is listed here: the node, what changed and + why, one bullet each. Write "none" when there is none. +4. The parts the project adds, such as its other approved artifacts, the + validation run, the review's scope and the findings it fixed, and what the + work found along the way. They cite a card by its ID instead of repeating + its reasons. + +Write each part as bullets under its bold name; a table's narrow columns bury +reasoning. + +## Workflow + +1. Settle the direction with the owner (Owner decisions). +2. Implement, change the tree and validate on a draft pull request. Examine + responsibilities, interfaces, representations and affected consumers for + design gaps and clear opportunities for a better design, even when the + current design is valid. Fix in-scope gaps and selected improvements; + record deferred ones in the maintained TODO with impact, uncertainty, + validation criterion and reopening condition, proportional to the work. + List each in the pull request with its disposition (fixed, deferred or + declined) and reason, so the owner sees it. +3. At completion, run DCR once, or the project's review that includes it. + Fix every finding, including those that change the tree or another + approved artifact; a fix that changes a decision becomes a ledger entry, + and one that changes the agreed scope goes into the handoff's status. + Review again only a fix that became a ledger entry or rewrote logic or + behavior beyond a local repair, and only what it touched; recheck other + fixes yourself and list them at handoff. +4. Hand off (Owner decisions). The owner rules on every open card. +5. Write the log entry, mark ready once the readiness check and the project's + CI pass, and leave the merge to the project's merge rules. + +## Design Correspondence Review (DCR) + +A separate, read-only reviewer that did not implement the change, normally a +small or mid-sized model with bounded inputs, reads the actual artifacts and +reports scope, revision, findings, evidence and uncertainty. It applies the +design checks G1–G3 and the correspondence checks DC1–DC4 below to the tree +diff, the complete work diff and the relevant existing nodes and ancestors. +A task without tree changes still gets DCR at completion. DCR approves +nothing. + +### Design checks + +G1. Decision test. Check each added or changed node against the node format +and leanness filters. Report descriptions without decisions, circular refusal +reasons, and choices or grounds that require the source record to understand. + +G2. Consistency scan. Check changed nodes against ancestors and siblings, +extending to related decisions as needed. A change governing a whole concept +requires reading its subtree. Report nodes read and conflicts, narrowings, or +broken dependencies, naming both sides. + +G3. Architectural fit. Check that structural choices received the Workflow +assessment when made or revised, and that the result is visible to the owner. +Report concrete gaps or clear improvement opportunities left without an +assessment or disposition, including deferred opportunities or their +validation missing from the maintained TODO. Do not demand speculative +generality or reconstruct a missing rationale after coding. + +### Correspondence: design and implementation + +Inputs: the agreed delivery scope, its design commitments including relevant +existing nodes and ancestors, the complete work diff, resulting artifacts, and +validation. Here, code means whichever artifact implements a decision, +including a specification or configuration. Extend into affected consumers as +needed. + +DC1. Decisions in code. For each changed region embodying a design choice, +name its node. Report a choice with no node as a missing tree change; +ordinary implementation steps need no record. + +DC2. Contradiction. Report code that contradicts a decision or implements a +refused alternative without a tree change that replaces the decision. + +DC3. Orphaned support. For deleted code, identify decisions that lose their +implementation. Report a retired approach missing its rejection rationale, +and rejected approaches still implemented. + +DC4. Missing or partial implementation. For each design commitment in scope, +identify support for its required behavior and conditions. Report missing or +partial paths, placeholders, and insufficient evidence; a related function +alone is not proof of completion. Exclude unrelated or explicitly deferred +designs unless the deferral contradicts the agreed scope or completion claim. + +## Lint + +`lint.py` checks form, not design quality. Its layout has one root node file +and optional child directory per concept, with `log.md` beside the roots. + + python3 -B <skill-directory>/lint.py --root <design-directory> --trees <concept> ... [--base <base>] [--require-approval] + +Without `--base` it checks form only. With `--base` it also prints node count, +depth and decision counts against the base, which a tree review reports. With +`--require-approval` it is the readiness check: when the tree differs from the +base, the newest log entry must be new, name every changed node and carry a +nonempty `Owner-approved:`. The field is an assertion that lint cannot +authenticate; the owner reads the log before merging. A `--base` must resolve +to a commit, and a caller must choose one that exposes the changes under +review: for a push to the main line, the revision before the push. In CI, +`sh <skill-directory>/review-base.sh EVENT REF PUSH_BEFORE` prints that base. + +## Translation: run on request + +Render the requested tree diff or subtree in the requested language, keeping +node names, paths, and code identifiers untranslated. Do not store the +translation in the repository; the tree is English only. diff --git a/design-tree/lint.py b/design-tree/lint.py new file mode 100644 index 0000000..085c64a --- /dev/null +++ b/design-tree/lint.py @@ -0,0 +1,299 @@ +#!/usr/bin/env python3 +"""Structural lint for the design tree: form, and with --require-approval the +readiness of a changed tree. Its messages say what it checks.""" +import argparse +import os +import re +import subprocess +import sys + +DECISION_MARKERS = (" because ", " instead of ") +LOG_ENTRY = re.compile(r"^## \d{4}-\d{2}-\d{2} \S") +LOG_REQUIRED = ("Nodes:", "Summary:") +OWNER_APPROVED = "Owner-approved:" +FORBIDDEN_HEADINGS = ("## Facts", "## Moves") +DATED_LINE = re.compile(r"^- 20\d\d-\d\d-\d\d") +REJECTED_ITEM = re.compile(r"^- (.+?): rejected because (\S.*)$") +DATE = re.compile(r"\b20[0-9]{2}-[0-9]{2}-[0-9]{2}\b") +FIELDS = ("Decision:", "Rejected:") + + +class Lint: + def __init__(self, root, trees): + self.root = root + self.trees = trees + self.errors = [] + self.nodes = {} # node path (relative to root, no .md) -> lines + self.decisions = 0 + self.rejected = 0 + self.base_metrics = None + + def err(self, where, msg): + self.errors.append(f"{where}: {msg}") + + # ---- discovery ----------------------------------------------------- + + def discover(self): + for tree in self.trees: + self.discover_tree(tree) + + def discover_tree(self, tree): + tree_md = os.path.join(self.root, tree + ".md") + tree_dir = os.path.join(self.root, tree) + if not os.path.isfile(tree_md): + self.err(tree_md, "missing root node") + return + self.nodes[tree] = self.read(tree_md) + if not os.path.isdir(tree_dir): + return + for dirpath, dirnames, filenames in os.walk(tree_dir): + rel_dir = os.path.relpath(dirpath, self.root) + parent_md = os.path.join(self.root, rel_dir + ".md") + if not os.path.isfile(parent_md): + self.err(rel_dir, "directory has no sibling node file") + for name in sorted(filenames): + path = os.path.join(dirpath, name) + rel = os.path.relpath(path, self.root) + if not name.endswith(".md"): + self.err(rel, "only node files (.md) belong under a tree") + continue + self.nodes[rel[:-3]] = self.read(path) + dirnames.sort() + + def read(self, path): + with open(path, "rb") as handle: + data = handle.read() + rel = os.path.relpath(path, self.root) + for number, raw in enumerate(data.split(b"\n"), 1): + if any(byte > 127 for byte in raw): + self.err(f"{rel}:{number}", "non-ASCII text; the tree is English only") + return data.decode("ascii", errors="replace").split("\n") + + # ---- node form ----------------------------------------------------- + + def check_nodes(self): + stems = {} + for path in self.nodes: + stem = path.rsplit("/", 1)[-1] + stems.setdefault(stem, []).append(path) + for stem, paths in stems.items(): + if len(paths) > 1: + self.err(stem, "node name used more than once: " + ", ".join(paths)) + for path, lines in sorted(self.nodes.items()): + self.check_node(path, lines, stems) + + def check_node(self, path, lines, stems, count=True): + where = path + ".md" + decisions = 0 + section = None + rejected = [] + prev = "start" # start | blank | field | item + seen_field = False + for number, line in enumerate(lines, 1): + loc = f"{where}:{number}" + if DATE.search(line): + self.err(loc, "a date; a node holds no history, only the live decision") + if not line.strip(): + section = None + prev = "blank" + continue + is_field = line.startswith(FIELDS) + is_item = line.startswith("- ") + if is_field: + if prev not in ("blank", "start"): + self.err(loc, "a field must be separated from the previous line by a blank line") + seen_field = True + prev = "field" + elif is_item: + if prev not in ("field", "item"): + self.err(loc, "a list item must directly follow its header or the previous item") + prev = "item" + else: + prev = "other" + if line.startswith("Decision:"): + decisions += 1 + low = line.lower() + if not any(marker in low for marker in DECISION_MARKERS): + self.err(loc, "decision has neither 'because' nor 'instead of'") + section = None + continue + if line.startswith("Rejected:"): + section = "rejected" + continue + if line.startswith(FORBIDDEN_HEADINGS) or DATED_LINE.match(line): + self.err(loc, "history belongs in git and the change log, not in a node") + continue + if line.startswith("- "): + body = line[2:] + if section == "rejected": + if not REJECTED_ITEM.match(line): + self.err(loc, "rejected entry needs '- <alternative>: rejected because <reason>'") + rejected.append(body) + continue + self.err(loc, "list item outside Rejected:") + continue + self.err(loc, "line outside the node template") + if decisions == 0: + self.err(where, "node has no Decision: line") + if count: + self.decisions += decisions + self.rejected += len(rejected) + + # ---- log ----------------------------------------------------------- + + def check_log(self): + path = os.path.join(self.root, "log.md") + if not os.path.isfile(path): + self.err("log.md", "missing change log") + return [] + lines = self.read(path) + entries = [] + current = None + for number, line in enumerate(lines, 1): + if line.startswith("## "): + if not LOG_ENTRY.match(line): + self.err(f"log.md:{number}", "entry heading must be '## YYYY-MM-DD <title>'") + current = {"line": number, "heading": line, "fields": {}} + entries.append(current) + continue + if current is None: + continue + for field in LOG_REQUIRED + (OWNER_APPROVED,): + if line.startswith(field): + current["fields"][field] = line[len(field):].strip() + first_line = {} + for entry in entries: + loc = f"log.md:{entry['line']}" + for field in LOG_REQUIRED: + if field not in entry["fields"] or not entry["fields"][field]: + self.err(loc, f"entry lacks {field}") + # A union merge keeps both sides of an entry edited on two branches. + if entry["heading"] in first_line: + self.err(loc, f"entry heading repeats log.md:{first_line[entry['heading']]}") + first_line.setdefault(entry["heading"], entry["line"]) + return entries + + def base_exists(self, base): + probe = subprocess.run(["git", "rev-parse", "--verify", "--quiet", + "--end-of-options", f"{base}^{{commit}}"], + cwd=self.root, capture_output=True, text=True) + return probe.returncode == 0 + + def check_diff(self, base, entries): + names = subprocess.run(["git", "diff", "--name-only", "-z", "--relative", base, "--", "."], + cwd=self.root, capture_output=True, text=True, + check=True).stdout.split("\0") + untracked = subprocess.run( + ["git", "ls-files", "--others", "--exclude-standard", "-z", "--", "."], + cwd=self.root, capture_output=True, text=True, check=True).stdout.split("\0") + names = list(dict.fromkeys(name for name in names + untracked if name)) + changed = set() + for rel in names: + if any(rel == tree + ".md" or rel.startswith(tree + "/") for tree in self.trees): + if rel.endswith(".md"): + changed.add(rel[:-3]) + if not changed: + return + log_rel = "log.md" + if log_rel not in names: + self.err("log.md", "tree changed since base but the change log did not") + return + if not entries: + self.err("log.md", "tree changed since base but the change log has no entry") + return + newest = entries[0] + base_log = subprocess.run(["git", "show", f"{base}:./{log_rel}"], + cwd=self.root, capture_output=True, text=True) + base_headings = set() + if base_log.returncode == 0: + base_headings = {line for line in base_log.stdout.splitlines() + if line.startswith("## ")} + if newest["heading"] in base_headings: + self.err("log.md", "tree changed since base but the newest log entry is not new") + approval = newest["fields"].get(OWNER_APPROVED) + if not approval: + self.err(f"log.md:{newest['line']}", + "newest entry for a tree change lacks a nonempty Owner-approved: field") + nodes = {node.strip() for node in newest["fields"].get("Nodes:", "").split(",") + if node.strip()} + for node in sorted(changed): + if node not in nodes: + self.err("log.md", f"changed node {node} is not named in the newest log entry") + + # ---- metrics ------------------------------------------------------- + + def measure_base(self, base): + """The same counts at the review base, so a tree diff review can report net change.""" + listing = subprocess.run(["git", "ls-tree", "-r", "--name-only", "-z", base, "--", "."], + cwd=self.root, capture_output=True, text=True, + check=True).stdout.split("\0") + counter = Lint(self.root, self.trees) + paths = [] + for rel in listing: + if not rel.endswith(".md"): + continue + if not any(rel == tree + ".md" or rel.startswith(tree + "/") for tree in self.trees): + continue + text = subprocess.run(["git", "show", f"{base}:./{rel}"], cwd=self.root, + capture_output=True, text=True, check=True).stdout + counter.check_node(rel[:-3], text.split("\n"), set()) + paths.append(rel[:-3]) + return {"nodes": len(paths), + "depth": max((path.count("/") for path in paths), default=0), + "decisions": counter.decisions, + "rejected": counter.rejected} + + def metrics(self): + depth = max((path.count("/") for path in self.nodes), default=0) + per_subtree = {} + for path in self.nodes: + parts = path.split("/") + key = parts[0] if len(parts) == 1 else "/".join(parts[:2]) + per_subtree[key] = per_subtree.get(key, 0) + 1 + base = self.base_metrics + + def show(name, value): + if base is None: + return f"{name}: {value}" + return f"{name}: {value} (base {base[name]}, {value - base[name]:+d})" + + print(f"{show('nodes', len(self.nodes))} {show('depth', depth)} " + f"{show('decisions', self.decisions)} {show('rejected', self.rejected)}") + for name, count in sorted(per_subtree.items()): + print(f" {name}: {count}") + + +def main(): + parser = argparse.ArgumentParser(description="structural lint for the design tree") + parser.add_argument("--root", default="design") + parser.add_argument("--trees", nargs="+", required=True, + help="top-level concept names to check") + parser.add_argument("--base", default=None) + parser.add_argument("--require-approval", action="store_true", + help="readiness: a tree changed since --base needs a new, approved log entry") + args = parser.parse_args() + lint = Lint(args.root, args.trees) + lint.discover() + lint.check_nodes() + entries = lint.check_log() + if args.require_approval and args.base is None: + lint.err("review base", "--require-approval needs --base") + if args.base is not None: + if lint.base_exists(args.base): + if args.require_approval: + lint.check_diff(args.base, entries) + lint.base_metrics = lint.measure_base(args.base) + else: + lint.err("review base", f"{args.base!r} does not resolve to a commit; cannot check tree changes") + if lint.errors: + for error in lint.errors: + print("error: " + error, file=sys.stderr) + print(f"design lint: {len(lint.errors)} error(s)", file=sys.stderr) + return 1 + lint.metrics() + print("design lint: ok") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/design-tree/review-base.sh b/design-tree/review-base.sh new file mode 100644 index 0000000..6750e6c --- /dev/null +++ b/design-tree/review-base.sh @@ -0,0 +1,42 @@ +#!/bin/sh +# Print the base a CI job passes to lint.py's --base: the revision preceding +# the change it checks. A push to main compares with the tip before the push, +# a manual run on main with the first parent, and any other ref with its fork +# point from origin/main. It assumes the main line is the branch main on the +# remote origin. Stdout contains only the resolved commit. +set -eu + +if [ "$#" -ne 3 ]; then + echo 'usage: review-base.sh EVENT REF PUSH_BEFORE' >&2 + exit 1 +fi +event=$1 +ref=$2 +before=$3 +case "$event" in + push|workflow_dispatch|pull_request) ;; + *) echo "unsupported design review event: $event" >&2; exit 1 ;; +esac + +if [ "$ref" = refs/heads/main ]; then + if [ "$event" = push ]; then + candidate=$before + else + # A manual main run checks the last revision against its first parent. + candidate=HEAD^ + fi +else + # New main-side commits are not changes made by this work branch; for a + # pull request HEAD is its merge commit, whose base is main's tip. + candidate=$(git merge-base origin/main HEAD) +fi + +if ! base=$(git rev-parse --verify --end-of-options "${candidate}^{commit}"); then + echo "cannot resolve design review base: $candidate" >&2 + exit 1 +fi +if [ "$ref" = refs/heads/main ] && [ "$base" = "$(git rev-parse HEAD)" ]; then + echo 'main design review base equals HEAD; refusing an empty comparison' >&2 + exit 1 +fi +printf '%s\n' "$base" diff --git a/design-tree/test_lint.py b/design-tree/test_lint.py new file mode 100644 index 0000000..6512b48 --- /dev/null +++ b/design-tree/test_lint.py @@ -0,0 +1,222 @@ +"""Regressions for the tree form check, the readiness check and the CI +base selection. + +These fixtures are maintained with the checked tools; replace them when those +tools are replaced. No fixture edits a live tree. +""" +from pathlib import Path +import subprocess +import sys +import tempfile +import unittest + + +SKILL = Path(__file__).resolve().parent +LINT = SKILL / "lint.py" +CI_BASE = SKILL / "review-base.sh" +DECISION = "Decision: Keep the fixture because it exercises the gate, instead of an unchecked change.\n" +LOG_HEADER = "# Design tree change log\n\n" +BASE_ENTRY = """## 2026-01-01 Baseline fixture + +Nodes: language + +Owner-approved: Fixture baseline approval. + +Summary: Establish the test tree. +""" + + +class TreeGateTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="design-lint-") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name) + self.git("init", "--quiet") + self.git("config", "user.name", "Design lint fixture") + self.git("config", "user.email", "design-lint@example.invalid") + self.write("design/language.md", DECISION) + self.write("design/log.md", LOG_HEADER + BASE_ENTRY) + self.base = self.commit("Baseline") + self.git("branch", "-M", "main") + self.git("update-ref", "refs/remotes/origin/main", self.base) + + def git(self, *args): + return subprocess.run( + ["git", "-c", "core.hooksPath=/dev/null", "-c", "commit.gpgsign=false", *args], + cwd=self.root, text=True, capture_output=True, check=True, + ).stdout.strip() + + def write(self, path, text): + target = self.root / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text, encoding="ascii") + + def commit(self, message): + self.git("add", ".") + self.git("commit", "--quiet", "-m", message) + return self.git("rev-parse", "HEAD") + + def change_tree(self): + self.write("design/language.md", DECISION.replace("Keep the fixture", "Change the fixture")) + + def log_change(self, approval="Fixture owner approved this revision.", nodes="language"): + field = "" if approval is None else f"Owner-approved: {approval}\n\n" + self.write("design/log.md", f"""# Design tree change log + +## 2026-01-02 Changed fixture + +Nodes: {nodes} + +{field}Summary: Record the fixture change. + +{BASE_ENTRY}""") + + def lint(self, base, require_approval=False): + command = [ + sys.executable, "-B", str(LINT), "--root", "design", + "--trees", "language", "--base", base, + ] + if require_approval: + command.append("--require-approval") + return subprocess.run( + command, + cwd=self.root, text=True, capture_output=True, + ) + + def ci_base(self, event, ref, before=""): + return subprocess.run( + ["sh", str(CI_BASE), event, ref, before], + cwd=self.root, text=True, capture_output=True, + ) + + def assert_rejected(self, result, diagnostic): + self.assertNotEqual(result.returncode, 0, result.stdout) + self.assertIn(diagnostic, result.stderr) + + def test_draft_tree_edit_passes_the_form_check(self): + self.change_tree() + self.write("design/language/new-node.md", DECISION) + result = self.lint(self.base) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_form_check_still_rejects_a_malformed_node(self): + self.write("design/language.md", "Decision: Keep the fixture as it is.\n") + self.assert_rejected(self.lint(self.base), "neither 'because' nor 'instead of'") + + def test_readiness_needs_a_base(self): + result = subprocess.run( + [sys.executable, "-B", str(LINT), "--root", "design", "--trees", "language", + "--require-approval"], + cwd=self.root, text=True, capture_output=True, + ) + self.assert_rejected(result, "--require-approval needs --base") + + def test_readiness_rejects_a_tree_edit_without_a_new_log(self): + self.change_tree() + self.assert_rejected(self.lint(self.base, require_approval=True), "change log did not") + + def test_readiness_rejects_an_untracked_node_without_a_new_log(self): + self.write("design/language/new-node.md", DECISION) + self.assert_rejected(self.lint(self.base, require_approval=True), "change log did not") + + def test_readiness_passes_an_approved_tree_change(self): + self.change_tree() + self.log_change() + result = self.lint(self.base, require_approval=True) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_missing_or_empty_approval_field_is_rejected(self): + self.change_tree() + for approval in (None, ""): + with self.subTest(approval=approval): + self.log_change(approval=approval) + self.assert_rejected(self.lint(self.base, require_approval=True), "nonempty Owner-approved:") + + def test_reused_old_log_entry_is_rejected(self): + self.change_tree() + self.write("design/log.md", LOG_HEADER + BASE_ENTRY.replace("Establish", "Update")) + self.assert_rejected(self.lint(self.base, require_approval=True), "newest log entry is not new") + + def test_repeated_log_entry_heading_is_rejected(self): + self.write("design/log.md", LOG_HEADER + BASE_ENTRY + "\n" + BASE_ENTRY.replace("Establish", "Build")) + self.assert_rejected(self.lint(self.base), "entry heading repeats log.md:3") + + def test_log_must_name_the_changed_node(self): + self.change_tree() + self.log_change(nodes="language/other") + self.assert_rejected(self.lint(self.base, require_approval=True), "language is not named") + + def test_missing_or_empty_explicit_base_fails_closed(self): + self.change_tree() + for base in ("missing-review-base", ""): + with self.subTest(base=base): + self.assert_rejected(self.lint(base), "review base") + + def test_noncommit_base_fails_closed(self): + tree = self.git("rev-parse", "HEAD^{tree}") + self.assert_rejected(self.lint(tree), "review base") + + def test_main_push_checks_before_even_when_origin_main_is_head(self): + self.change_tree() + head = self.commit("Unlogged tree edit") + self.git("update-ref", "refs/remotes/origin/main", head) + # This is the old CI wiring's vacuous comparison. + self.assertEqual(self.lint("origin/main", require_approval=True).returncode, 0) + result = self.ci_base("push", "refs/heads/main", self.base) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), self.base) + self.assert_rejected( + self.lint(result.stdout.strip(), require_approval=True), "change log did not", + ) + + def test_main_push_rejects_missing_before_instead_of_using_head(self): + for before in ("", "0" * 40, "missing-before"): + with self.subTest(before=before): + self.assert_rejected( + self.ci_base("push", "refs/heads/main", before), "cannot resolve", + ) + + def test_main_push_rejects_a_self_comparison(self): + self.assert_rejected(self.ci_base("push", "refs/heads/main", self.base), "equals HEAD") + + def test_manual_main_run_uses_the_first_parent(self): + self.change_tree() + self.commit("Unlogged tree edit") + result = self.ci_base("workflow_dispatch", "refs/heads/main") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), self.base) + + def test_work_branch_uses_its_fork_point_not_new_main_changes(self): + self.change_tree() + self.commit("Work branch edit") + self.git("checkout", "--quiet", "-b", "new-main", self.base) + self.write("unrelated.txt", "Main advanced.\n") + newer_main = self.commit("Unrelated main change") + self.git("update-ref", "refs/remotes/origin/main", newer_main) + self.git("checkout", "--quiet", "main") + result = self.ci_base("push", "refs/heads/work", self.base) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), self.base) + + def test_new_work_branch_can_share_the_main_tip(self): + result = self.ci_base("push", "refs/heads/work", "0" * 40) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), self.base) + + def test_work_branch_with_no_main_base_fails(self): + self.git("update-ref", "-d", "refs/remotes/origin/main") + self.assertNotEqual(self.ci_base("push", "refs/heads/work").returncode, 0) + + def test_pull_request_uses_the_merge_base_of_main(self): + self.change_tree() + self.commit("Pull request edit") + result = self.ci_base("pull_request", "refs/pull/1/merge") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), self.base) + + def test_unhandled_event_fails(self): + self.assert_rejected(self.ci_base("unknown-event", "refs/heads/main"), "unsupported") + + +if __name__ == "__main__": + unittest.main() From 1647bdcd1dc3c3342419c5eba4dce15bc6813ff7 Mon Sep 17 00:00:00 2001 From: Bai Ming <mbbill@Bais-MacBook-Air-M5.local> Date: Mon, 5 Oct 2026 22:50:07 -0700 Subject: [PATCH 2/2] Put the skill at the repository root and use it as a submodule A project mounts this repository at design/skill, so its paths, links and Makefile targets stay as they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .github/workflows/test.yml | 2 +- README.md | 38 +++++++++++++------- design-tree/SKILL.md => SKILL.md | 0 design-tree/lint.py => lint.py | 0 design-tree/review-base.sh => review-base.sh | 0 design-tree/test_lint.py => test_lint.py | 0 6 files changed, 27 insertions(+), 13 deletions(-) rename design-tree/SKILL.md => SKILL.md (100%) rename design-tree/lint.py => lint.py (100%) rename design-tree/review-base.sh => review-base.sh (100%) rename design-tree/test_lint.py => test_lint.py (100%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cff35a9..c5cb236 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,4 +11,4 @@ jobs: steps: - uses: actions/checkout@v4 - name: Test the lint and the CI base selection - run: python3 -B -m unittest discover -s design-tree -p 'test_lint.py' -v + run: python3 -B -m unittest discover -s . -p 'test_lint.py' -v diff --git a/README.md b/README.md index f2626f5..ecf360a 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ Summary: Layout lengths become 1/64-pixel integers; the measurements are in research/investigations/vocabulary/DESIGN.md. ``` -[`design-tree/SKILL.md`](design-tree/SKILL.md) is the full procedure: the +[`SKILL.md`](SKILL.md) is the full procedure: the node format, what counts as a decision, how the owner's decisions are gathered into decision cards at handoff, the log format and the review checks. @@ -79,20 +79,29 @@ checks. | File | Purpose | |---|---| -| `design-tree/SKILL.md` | The skill, loaded by the agent when a task matches its description | -| `design-tree/lint.py` | Form check of a tree and, with `--require-approval`, the readiness check | -| `design-tree/review-base.sh` | Picks the base revision a CI job compares the tree with | -| `design-tree/test_lint.py` | Tests for the two scripts above | +| `SKILL.md` | The skill, loaded by the agent when a task matches its description | +| `lint.py` | Form check of a tree and, with `--require-approval`, the readiness check | +| `review-base.sh` | Picks the base revision a CI job compares the tree with | +| `test_lint.py` | Tests for the two scripts above | The scripts need Python 3, Git and a POSIX shell. ## Using it in a project -A project keeps its own copy of `design-tree/`, which it does not edit: -changes are made here, so every project runs the same skill. +A project adds this repository as a Git submodule, which pins the commit it +uses. The skill is changed only here, and a project adopts a change by moving +its pin. -1. **Copy it** into the project, for example as `design/skill/`, and name - the Design-skill commit it came from in the commit message. +1. **Add the submodule**, for example at `design/skill`: + + ```sh + git submodule add https://github.com/Ming-Research/Design-skill.git design/skill + ``` + + A fresh clone then needs `git clone --recurse-submodules`, or + `git submodule update --init` afterwards, and CI checks out submodules + (`submodules: true` in `actions/checkout`). Until the submodule is + initialized, the agents do not see the skill. 2. **Expose it to the agents.** From the project's root, link it where Claude Code and Codex look for skills: @@ -133,9 +142,14 @@ changes are made here, so every project runs the same skill. ## Changing the skill -Change it here, by pull request, with the tests passing. Then copy the merged -revision into each project in that project's own pull request, which names -the Design-skill commit it adopts. +Change it here, by pull request, with the tests passing. A project then +moves its submodule to the merged commit in its own pull request, which names +the Design-skill revision it adopts and why: + +```sh +git -C design/skill fetch origin && git -C design/skill checkout <commit> +git add design/skill +``` ## Projects using it diff --git a/design-tree/SKILL.md b/SKILL.md similarity index 100% rename from design-tree/SKILL.md rename to SKILL.md diff --git a/design-tree/lint.py b/lint.py similarity index 100% rename from design-tree/lint.py rename to lint.py diff --git a/design-tree/review-base.sh b/review-base.sh similarity index 100% rename from design-tree/review-base.sh rename to review-base.sh diff --git a/design-tree/test_lint.py b/test_lint.py similarity index 100% rename from design-tree/test_lint.py rename to test_lint.py