Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 38 additions & 37 deletions .agents/skills/mx-api-classify-roundtrip-failures/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,39 @@ user-invocable: true
# Explain the `mx::api` round-trip

`mx::api` is a deliberate subset of MusicXML, and the comparison is strict full-DOM, so most corpus
files diverge somewhere. This skill turns the measured divergences into a plain-English answer to two
questions: what is broken, and what should we fix next to land the most files into the round-trip
corpus with the fewest software changes.
files diverge somewhere. This skill turns the measured divergences into a plain-English answer to
two questions: what is broken, and what should we fix next to land the most files into the
round-trip corpus with the fewest software changes.

It is the read-out layer on top of the failure classifier (issue #211/#212): the classifier produces
a machine-readable `build/api/classified.json`; this skill interprets it for a human.

## What the classifier reports (and what it does not)

Classification is purely **measured** from each expected/actual pair. It does **not** consult
`data/api.features.xml` or any record of what `mx::api` was believed to "support" — whether a drop is
Classification is purely measured from each expected/actual pair. It does not consult
`data/api.features.xml` or any record of what `mx::api` was believed to "support". Whether a drop is
intended is a present-day human decision (#214), not something the tool asserts. So do not describe
any drop as "by-design" on the classifier's authority; report what was dropped and let the human
decide.

Each difference is a **signature**, and a file's **distance** to passing is its count of unique
signatures:
Each difference is a signature, and a file's distance to passing is its count of unique signatures:

| signature | meaning |
|-----------|---------|
| `drop:<tag>` | a tag in expected, missing from actual |
| `add:<tag>` | a spurious tag only in actual (a bug) |
| `value:<tag>` | a paired element whose text value differs |
| `value:<tag>@<attr>` | a paired element whose attribute value differs |
| `attr:<tag>@<attr>` | an attribute present on only one side |
| `reorder:<parent>` | a parent with matching children in a different order |
| signature | meaning |
|----------------------|------------------------------------------------------|
| `drop:<tag>` | a tag in expected, missing from actual |
| `add:<tag>` | a spurious tag only in actual (a bug) |
| `value:<tag>` | a paired element whose text value differs |
| `value:<tag>@<attr>` | a paired element whose attribute value differs |
| `attr:<tag>@<attr>` | an attribute present on only one side |
| `reorder:<parent>` | a parent with matching children in a different order |

Per-file `status` is `PASS` / `FAIL` / `CRASH`. A `FAIL` with no `reorder:` signature is a
**candidate**: the first-pass worklist targets these, since reorders are expected `mx::api` behavior
to be absorbed in test normalization later (#214).
candidate: the first-pass worklist targets these, since reorders are expected `mx::api` behavior to
be absorbed in test normalization later (#214).

## Procedure

### Step 1 — produce the data
### Step 1: produce the data

If `build/api/roundtrip-dump/` is empty or stale (api/impl changed since it was written), run:

Expand All @@ -61,18 +60,18 @@ Then always run:
make api-roundtrip-classify
```

Read the stdout summary it prints -- status counts, the distance histogram, the ranked worklist,
and the greedy batch plan. That worklist *is* the headline answer to "what should we fix next."
Read the stdout summary it prints -- status counts, the distance histogram, the ranked worklist, and
the greedy batch plan. That worklist is the headline answer to "what should we fix next."

### Step 2 — mine `build/api/classified.json`
### Step 2: mine `build/api/classified.json`

These read-only analyses expand on the stdout summary. Adjust the path if `--out` was overridden.

The worklist (already in the stdout summary) ranks signatures by candidate files unblocked
(`sole_blocker` = files this fix flips green on its own; `files_blocked` = candidate files that
include it).

Instant wins — candidate files one fix away from passing (distance 1):
Instant wins: candidate files one fix away from passing (distance 1):

```
python3 - <<'PY'
Expand All @@ -86,7 +85,8 @@ PY
Small fix-sets -- the greedy set-cover batch plan is already computed in `d["batch_plan"]` (rows
have `fix`/`added_files`/`cumulative_files`; also printed in the stdout summary).

Crash cluster (highest severity — no output at all) — group by kind to find the common root:
Crash cluster (highest severity: the run does not produce output). Group by kind to find the common
root:

```
python3 - <<'PY'
Expand All @@ -100,7 +100,7 @@ for kind, f in crashes[:20]:
PY
```

Reorder-blocked files (deferred to #214) — how many, and at which parents:
Reorder-blocked files (deferred to #214): how many, and at which parents:

```
python3 - <<'PY'
Expand All @@ -116,27 +116,28 @@ PY

To drill into one file, look at its record (`signatures`, `sample_paths`, `distance`) and diff the
pair directly:
`diff build/api/roundtrip-dump/<flat>.expected.xml build/api/roundtrip-dump/<flat>.actual.xml`
where `<flat>` is the corpus path with `/` replaced by `__`.
`diff build/api/roundtrip-dump/<flat>.expected.xml build/api/roundtrip-dump/<flat>.actual.xml` where
`<flat>` is the corpus path with `/` replaced by `__`.

### Step 3 — write the explanation
### Step 3: write the explanation

Synthesize the findings into plain language, ordered by what grows the corpus fastest. Use this
structure (fill the numbers and signatures from Step 2; do not invent them):

1. Frame it: strict full-DOM, so a file passes only when *every* signature is resolved. The goal is
to land files with the fewest software changes.
2. Crashes (`status="CRASH"`). Highest severity: no output at all. Name the cluster (the crash kind
usually points at one feature). These are bugs.
3. Instant wins: the distance-1 candidates and the top `sole_blocker` signatures — one fix each flips
1. Frame it: strict full-DOM, so a file passes only when all of its signatures are resolved. The
goal is to land files with the fewest software changes.
2. Crashes (`status="CRASH"`). Highest severity: the run does not produce output. Name the cluster
(the crash kind usually points at one feature). These are bugs.
3. Instant wins: the distance-1 candidates and the top `sole_blocker` signatures. One fix each flips
a file green now. This is the front of the worklist.
4. Small fix-sets: the distance-2/3 candidates and the union of signatures that would unblock a batch
— "add these N features → these M files pass."
4. Small fix-sets: the distance-2/3 candidates and the union of signatures that would unblock a
batch, stated as "add these N features and these M files pass."
5. Reorder-blocked: count them, name the top `reorder:` parents, and note they are deferred to test
normalization (#214), not part of the first-pass worklist.
6. High-frequency drops that are *not* sole blockers: large `files_blocked` with low `sole_blocker`
means the fix helps many files but flips none alone — flag as enabling, lower immediate yield.
6. High-frequency drops that are not sole blockers: large `files_blocked` with low `sole_blocker`
means the fix helps many files but does not flip any file alone. Flag it as enabling, with a
lower immediate yield.

Be honest about caveats: the comparison is strict full-DOM, and if the pinned baseline
(`roundtrip-baseline.txt`) is ungrown, almost the whole corpus shows as failing — these are the raw
(`roundtrip-baseline.txt`) is ungrown, almost the whole corpus shows as failing. These are the raw
landscape, not a regression.
2 changes: 1 addition & 1 deletion .agents/skills/mx-api-doctrine/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ Do not let an edge case complicate the mainline type -- and do not drop it. Cano
`<tied type="let-ring">` is its own optional `TieLetRing` struct, not a third tie boolean
(`NoteData.h`). Test: does the 95% use read as one obvious field, while the 5% is still expressible?

### 7. Fidelity knobs must default to "automatic" and be ignorable
### 7. Fidelity fields must default to "automatic" and be ignorable

When byte-level round-trip needs a source quirk recorded, add a field whose default applies the
sensible rule; only the reader sets it. Canonical: `ClefData::writeStaffNumber`,
Expand Down
1 change: 1 addition & 0 deletions .agents/skills/mx-comments/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ description: >
argument-hint: "<prompt>"
disable-model-invocation: false
user-invocable: true
llm-edit: false
---
# /mx-comments

Expand Down
4 changes: 3 additions & 1 deletion .agents/skills/mx-open-issue/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ description: >
argument-hint: "<prompt>"
disable-model-invocation: false
user-invocable: true
llm-edit: false
---
# /mx-open-issue

Expand Down Expand Up @@ -34,7 +35,8 @@ to related issues and/or PRs (if any are related). If it is a child of another i
tracking issue body to add it to the list of tracked issues and also make it a sub-issue of the
tracker.

Do not pretend to be the user. Instead start the issue like this to make it clear that it was AI written (and add the `ai` label):
Do not pretend to be the user. Instead start the issue like this to make it clear that it was AI
written (and add the `ai` label):

```markdown
> Written on behalf of the user by AI...
Expand Down
11 changes: 6 additions & 5 deletions .agents/skills/mx-open-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ description: >
argument-hint: "<prompt>"
disable-model-invocation: false
user-invocable: true
llm-edit: false
---
# /mx-open-pr

Expand All @@ -14,8 +15,8 @@ it was previously documented by an api-feature-audit).

Use `gh` for GitHub interactions.

Search the open issues for one that this PR would close or that it is related to. Make a list of issues
that it closes and issues that is related to. It might also be related to other PRs.
Search the open issues for one that this PR would close or that it is related to. Make a list of
issues that it closes and issues that is related to. It might also be related to other PRs.

When writing the PR, the title should be lowercase and start with one keyword, e.g.
- build: add cache to make fmt for ci
Expand All @@ -36,8 +37,8 @@ issues closed and issues/PRs related (if any).

No self-attribution in the PR body; add the `ai` label instead.

Do not use excessive bold and italics in the PR body. Go easy on the keywords since they will add
a lot of backtick formatting as well.
Do not use excessive bold and italics in the PR body. Go easy on the keywords since they will add a
lot of backtick formatting as well.

Template/sample:

Expand All @@ -62,4 +63,4 @@ double sharps are now fully supported in `mx::api`.
- Closes #123
- Progresses #456
- Fixes a bug introduced in #789
```
```
31 changes: 16 additions & 15 deletions .github/instructions/api-headers.instructions.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
---
applyTo: "src/include/mx/api/**"
---

These are the public `mx::api` headers: plain-old-data structs that re-shape MusicXML into a
simpler model (doctrine: `.claude/skills/mx-api-doctrine/SKILL.md`). Review for:
These are the public `mx::api` headers: plain-old-data structs that re-shape MusicXML into a simpler
model (doctrine: `.claude/skills/mx-api-doctrine/SKILL.md`). Review for:

- No `mx::core` type may appear in a public header.
- New absent-able fields must be `std::optional<T>`. New absent-able enums must instead have an
`unspecified` first enumerator (including uses of the ternary `Bool`). Reject new `-1`
sentinels and new `bool is...Specified` / `has...` flags; do not ask for migration of existing
ones (tracked in issue #249).
`unspecified` first enumerator (including uses of the ternary `Bool`). Reject new `-1` sentinels
and new `bool is...Specified` / `has...` flags; do not ask for migration of existing ones (tracked
in issue #249).
- Every new field needs a default value and a matching `MXAPI_EQUALS_MEMBER(field)` line
(`MXAPI_DOUBLES_EQUALS_MEMBER` for doubles) in the type's equality block. A missing line
silently exempts the field from equality and round-trip checks -- flag it as a bug.
- New data must not mirror MusicXML's raw shape when the element is stateful, flat,
duplicated, or id-linked; check the change against the principles doc.
(`MXAPI_DOUBLES_EQUALS_MEMBER` for doubles) in the type's equality block. A missing line silently
exempts the field from equality and round-trip checks -- flag it as a bug.
- New data must not mirror MusicXML's raw shape when the element is stateful, flat, duplicated, or
id-linked; check the change against the principles doc.
- A new positioned-in-a-measure type needs `int tickTimePosition`; durations are in ticks.
- No UB or exceptions reachable through the public interface; a failed precondition must never
throw. Flag new `Result` usage outside `MusicXml`, unchecked `std::get`, or accessors
that return references guarded only by a precondition.
throw. Flag new `Result` usage outside `MusicXml`, unchecked `std::get`, or accessors that return
references guarded only by a precondition.
- Kind-specific payloads use the choice-class pattern (`TimeChoice.h`, `MarkDataChoice.h`), not
loose fields that apply only to some kinds.
- Comments must serve a user authoring or reading notation: crisp, semantics-first, explaining
how MusicXML encodes the feature. Flag round-trip framing ("the source", "preserved") except
on true fidelity knobs, notes about what is not modeled, and separator/banner comments.
- Comments must serve a user authoring or reading notation: crisp, semantics-first, explaining how
MusicXML encodes the feature. Flag round-trip framing ("the source", "preserved") except on true
fidelity fields
- Flag notes about what is not modeled or missing. Saying what is not there is not as helpful as
saying what is.
2 changes: 1 addition & 1 deletion .github/instructions/gen.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ C++ (`gen/cpp`, the product), Go, C, JSON Schema. Review against its cardinal ru
- Language-agnosticism: no language knowledge in any `*.py` outside a target's own directory.
Reject language tables, per-language branches, or target-name checks in generator Python;
language facts belong in the target's `config.toml` and `templates/`.
- The IR is a pure function of the XSD: no configurable knobs in `gen/ir/` (the `sounds.xml`
- The IR is a pure function of the XSD: no configurable fields in `gen/ir/` (the `sounds.xml`
fold is the one documented exception).
- Templates stay dumb: naming/structural decisions belong in Plates or press context, not in
template text. The Mustache engine has no expressions or filters -- reject attempts to add
Expand Down
17 changes: 16 additions & 1 deletion deslag.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,18 @@ schema_version = 2
deslag_version = "0.0.6"

[md]
globs = ["/AGENTS.md", "/docs/**/*.md"]
# .claude/skills is a symlink to .agents/skills, so only the .agents path is listed.
globs = ["/AGENTS.md", "/docs/**/*.md", "/.agents/skills/**/*.md"]

# The skills whose frontmatter says `llm-edit: false`. The owner does not want prose edits made
# automatically, so deslag does not check them. Each is listed by path; add a skill here only when
# its frontmatter carries the marker.
exclude = [
"/.agents/skills/mx-api-doctrine/SKILL.md",
"/.agents/skills/mx-comments/SKILL.md",
"/.agents/skills/mx-open-issue/SKILL.md",
"/.agents/skills/mx-open-pr/SKILL.md",
]

# The byte budget.
[md.lints.max_size_bytes]
Expand Down Expand Up @@ -42,6 +53,8 @@ load-bearing = 'say it in plain English, such as "important", "fundamental" or "
"lands on" = 'say what happens, such as "is included in" or "is released by"'
"it lands" = 'say what happens, such as "it is merged" or "it ships"'
byte-identical = 'say "identical", or name the check that shows it'
knob = "use the correct term for what it is instead, such as argument, parameter, attribute, field or constant"
knobs = "use the correct term for what they are instead, such as arguments, parameters, attributes, fields or constants"

# The longest a paragraph or list item may be.
[md.lints.density]
Expand Down Expand Up @@ -107,6 +120,8 @@ load-bearing = 'say it in plain English, such as "important", "fundamental" or "
"lands on" = 'say what happens, such as "is included in" or "is released by"'
"it lands" = 'say what happens, such as "it is merged" or "it ships"'
byte-identical = 'say "identical", or name the check that shows it'
knob = "use the correct term for what it is instead, such as argument, parameter, attribute, field or constant"
knobs = "use the correct term for what they are instead, such as arguments, parameters, attributes, fields or constants"

[cpp.lints.density]
max_item_chars = 350
Expand Down
2 changes: 1 addition & 1 deletion gen/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ If a template needs new dispatch, add a discriminant flag or context field in
specific data (namespace, package, fn_prefix) belongs under `[vars]` in `config.toml` and passes
through as `{{target.vars.namespace}}`.

**2. The IR is a pure function of the XSD.** No configurable knobs may enter `gen/ir/`. The
**2. The IR is a pure function of the XSD.** No configurable parameters may enter `gen/ir/`. The
`sounds.xml` fold is the one documented exception (opt-in input selection; runs before IR is handed
to Plates).

Expand Down
2 changes: 1 addition & 1 deletion gen/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ XSD -> XSD model -> IR -> Plates -> press -> output files

2. **Lower** (`gen/ir/`): resolves that into the intermediate representation (IR): a flat,
fully-named, dependency-ordered model in code-generation terms. A pure function of the XSD
(see Design principles). No configurable knobs.
(see Design principles). No configurable parameters.

3. **Project** (`gen/plates/`): binds the IR to one target. Every per-target decision --
identifier casings, renames, primitive type mappings, emit strategies, file layout -- is made
Expand Down
4 changes: 2 additions & 2 deletions gen/press/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ def _parse(tokens: list, name: str) -> list:
class Press:
"""Renders Mustache templates. `partials` maps a partial name to its
template text (a dict, or a callable for file-backed loading). The
`strict` and `escape` knobs exist for the spec conformance suite; the
production pipeline uses the defaults (strict, verbatim)."""
`strict` and `escape` parameters exist for the spec conformance suite;
the production pipeline uses the defaults (strict, verbatim)."""

def __init__(
self,
Expand Down
2 changes: 1 addition & 1 deletion gen/tests/test_press.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
1. Spec conformance: every case of the five vendored core modules of the
official Mustache spec suite (gen/tests/mustache_spec/). The suite is
run with the spec's own semantics (lenient misses, HTML escaping) via
the engine's test-affordance knobs, so the resolution, section,
the engine's test-affordance parameters, so the resolution, section,
standalone-whitespace, set-delimiter, and partial-indentation machinery
is pinned to the spec with zero skips.
2. The three documented deviations, under production defaults: missing
Expand Down
Loading