docs: audit agent instructions and skills for accuracy and brevity - #292
Merged
Conversation
- AGENTS.md: quickstart, mx::api presence/absence conventions (std::optional for new fields, unspecified enumerators for enums incl. Bool), accurate 'make test' scope, soften anonymous-namespace rule to match reality, condense Docker/audit/corert sections, fix typos - gen/AGENTS.md: fix nonexistent 'make test-cpp' target, unify commit policy - gen/README.md: point at gen/AGENTS.md instead of duplicating commands - README.md: fix non-compiling createFromScore example (Result<int>.value()), ARGS filter example, 'make dev' description - audit/data READMEs, Makefile, CI: fix skill-name references, add classify.py to layout table, fix garbled sentences, drop stale 829 pin count comment - code: add TODOs for 5 suspected bugs (arranger/publisher clobber lyricist, time-mod normal-type && condition, filename off-by-one, silent normal-type drop for tuplets without <tuplet> marks, ambiguous __ path flattening) and 9 clarifying comments at verified confusing spots
Corrects stale claims and trims redundancy across .claude/skills per the instructions audit; a follow-up commit completes the remaining skill fixes.
- skills: complete audit fixes (arch trimmed of irrelevant generic sections, stale examples replaced, optionality guidance in add-feature step-2 now matches the std::optional / unspecified convention) - docs/ai/design/api-design-principles.md: seven principles describing how mx::api re-shapes MusicXML (stateless time, containment over labels, one-fact-one-field, no neighbor-dependent meaning, denormalized state, quarantined edge cases, ignorable fidelity knobs), referenced from AGENTS.md and the mx-api-add-feature skill - NoteData.h: correct stale tie comment (writer emits both tie and tied) - DurationData.h: TODO on dead isTied field
Six .github/instructions/*.instructions.md files gated by applyTo globs: public api headers (presence/absence conventions, equality macros), impl (read/write symmetry, silent-drop and sentinel contracts), generated core (never review or hand-edit generated code), gen (cardinal rules), repo-wide C++ (anonymous namespaces, formatter-owned layout), and corpus/tests (pinned counts, markers, baseline discipline).
Agents reliably read AGENTS.md (SessionStart hook) but often skip linked docs; carry the seven rules inline, keep the full doc for examples and per-rule tests.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.7% | 6101 / 7754 |
| Functions | 64.3% | 2091 / 3250 |
| Branches | 47.8% | 5182 / 10834 |
Core HTML report | API HTML report
Commit 124e3d2314d151486e2b03cb7a42b8b582481255.
gen-quality
|
arch -> mx-architecture, grill-me -> mx-grill-me, open-mx-issue -> mx-open-issue, open-mx-pr -> mx-open-pr, project -> mx-project, questions -> mx-questions. Cross-references updated; open-issue/open-pr descriptions gain trigger phrases (report a bug, submit as PR). AGENTS.md layout notes the convention.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.7% | 6101 / 7754 |
| Functions | 64.3% | 2091 / 3250 |
| Branches | 47.8% | 5182 / 10834 |
Core HTML report | API HTML report
Commit 4cda961ba1756f5905628b9ee04c21a71296ed00.
gen-quality
|
Job display names now state the suites executed (gen gates, api mxtest + round-trip, core corert/unit/xmllint/probes, Go+C corert targets, coverage, macOS core+api, swift build-only); the coverage and macos names previously claimed core-only while also running api suites. Job keys are unchanged. Comments note what each job deliberately skips.
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.7% | 6101 / 7754 |
| Functions | 64.3% | 2091 / 3250 |
| Branches | 47.8% | 5182 / 10834 |
Core HTML report | API HTML report
Commit 6485b6e81b62e528ea776ee82d0e12e665bca53a.
gen-quality
|
webern
commented
Jul 4, 2026
- mx-open-pr template gains a Human Summary section for the maintainer to fill in - api-headers review instructions: comments should serve humans and coding agents, crisp and focused on the surprising - cpp review instructions: comments state constraints and effects hard to ascertain from the code
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.7% | 6117 / 7776 |
| Functions | 64.3% | 2091 / 3252 |
| Branches | 47.9% | 5206 / 10866 |
Core HTML report | API HTML report
Commit 41f7c9f3b46c8d45c1a96c51094b8a1edc92553f.
gen-quality
|
2 tasks
webern
added a commit
that referenced
this pull request
Jul 12, 2026
## Human Summary
I asked Fable, while I still have access to it, to improve Agent
performance with better instructions putting them in places more likely
to be read and incorporating some of my latest pet-peeves.
## Summary
Agents were not reading docs/ai/design/api-design-principles.md, and
several recurring review
themes were written down nowhere. This PR moves the doctrine to where
agents actually look and
captures the missing rules, without growing AGENTS.md (it shrinks by 8
lines).
- New mx-api-doctrine skill whose description says it MUST be read
before designing or changing
anything in the api layer. It contains the seven design principles
(moved verbatim from the
deleted doc) plus the previously unwritten doctrine:
- Audience: the api is for people creating music and building,
modifying, and interpreting
MusicXML files. Round-tripping is how we develop the api, not what it is
for. Header comments
explain notation semantics and how MusicXML encodes them;
"source"/"preserved" framing is
allowed only on true fidelity knobs. No documenting what is not modeled,
no banner comments
(from review feedback on #326, #333, #321).
- Failure model: misuse is made unrepresentable or degrades to a
documented fallback. UB must be
unreachable from the public interface, nothing throws across the api
boundary (never for a
failed precondition), and Result stays quarantined at the
DocumentManager I/O boundary.
- Choice types: the TimeChoice / MarkDataChoice pattern for either/or
payloads (copy-returning
accessors, default-constructed wrong-kind fallback, auto-collapse,
common case kept prominent
in its own header), from the #321 redesign and the #330 review.
- AGENTS.md digest replaced with a pointer to the skill; the add-feature
skill and the Copilot
api-headers review instructions repointed and extended with the same
rules.
- Deleted the mx-architecture skill: generic architect boilerplate that
wasn't helping. Its few
mx-specific facts live elsewhere (gen design in gen/DESIGN.md; the
error-handling stance is now
concrete doctrine in the new skill).
Net -33 lines repo-wide.
## Testing
- [x] No dangling references: grep for api-design-principles and
mx-architecture is clean
- [x] Docs/skills only; no C++ or build changes, so no test suites apply
## References
- Related: #292 (created the principles doc and the AGENTS.md digest
this replaces)
- Captures review feedback from #321, #326, #330, #333
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
To get the most out of limited Fable preview, I asked it to take a pass over the repository looking to improve its legibility to coding agents on a few dimensions:
AI Summary
What the audit found and fixed:
make testruns everything and banned anonymous namespaces "anywhere" while 45 files contain them, the README hello-world example did not compile (Result<int>needs.value()), and several skills described a repo state that is months stale (baseline "pins one file" vs ~140, "segno is never written", a worked bug example that is already fixed).std::optional; absent-able enums keep anunspecifiedfirst enumerator (including the ternaryBool); no new-1/is...Specifiedpatterns and no mass-migration. Codifies the convention from the feat(api): support lv (let-ring) ties #286 review discussion.&&that drops explicit normal-type, an off-by-one in test filename stripping, silent normal-type loss for tuplets without tuplet notations, ambiguous__path flattening in the classifier, and the deadDurationData::isTiedfield. Each TODO is an issue candidate; happy to file them.Testing
python3 -m py_compileon touched PythonemitSegnoexists, baseline length,batch_planoutput)References
isSpecifiedbool pairs with an explicit optional value type #249 (isSpecified -> optional direction now documented), Sentinel values should have constexpr names. #282 (sentinel naming), feat(api): support lv (let-ring) ties #286 (review discussion that set the optional/unspecified convention)Generated by Claude Code