docs(skills): clarify that keyed-merge requires a single identifier key per array
Review feedback (PR #2284) flagged that the merge-rules wording was ambiguous: "every item has a `code` or `id` field" could reasonably be read as "each item individually has at least one of the two", allowing arrays to mix `code` and `id` across items. The resolver has always required all items share the *same* identifier key (all `code`, or all `id`). Mixed arrays fall through to append — intentional, because mixing identifier keys within one array is a schema smell and any guess about which key should merge creates a worse trap than the append-fallback. Clarified in three places: - Merge-rules table in customize-bmad.md: "every item shares the **same** identifier field" - `code`/`id` convention paragraph: "pick **one** convention ... and stick with it across the whole array" - Resolver docstring and `_detect_keyed_merge_field` docstring: explicit note that mixed arrays fall through with rationale No behavior change.
This commit is contained in:
parent
f51159bf83
commit
80ba89eb52
|
|
@ -44,14 +44,14 @@ The resolver applies four structural rules. Field names are never special-cased
|
||||||
|---|---|
|
|---|---|
|
||||||
| Scalar (string, int, bool, float) | Override wins |
|
| Scalar (string, int, bool, float) | Override wins |
|
||||||
| Table | Deep merge (recursively apply these rules) |
|
| Table | Deep merge (recursively apply these rules) |
|
||||||
| Array of tables where every item has a `code` or `id` field | Merge by that key — matching keys **replace in place**, new keys **append** |
|
| Array of tables where every item shares the **same** identifier field (every item has `code`, or every item has `id`) | Merge by that key — matching keys **replace in place**, new keys **append** |
|
||||||
| Any other array (scalars, mixed, tables without `code`/`id`) | **Append** — base items first, then team items, then user items |
|
| Any other array (scalars; tables with no identifier; arrays that mix `code` and `id` across items) | **Append** — base items first, then team items, then user items |
|
||||||
|
|
||||||
**No removal mechanism.** Overrides cannot delete base items. If you need to suppress a default menu item, override it by `code` with a no-op description or prompt. If you need to restructure an array more deeply, fork the skill.
|
**No removal mechanism.** Overrides cannot delete base items. If you need to suppress a default menu item, override it by `code` with a no-op description or prompt. If you need to restructure an array more deeply, fork the skill.
|
||||||
|
|
||||||
#### The `code` / `id` convention
|
#### The `code` / `id` convention
|
||||||
|
|
||||||
BMad uses `code` (short identifier like `"BP"` or `"R1"`) and `id` (longer stable identifier) as merge keys on arrays of tables. If you author a custom array-of-tables that should be replaceable-by-key rather than append-only, give every item a `code` or `id` field.
|
BMad uses `code` (short identifier like `"BP"` or `"R1"`) and `id` (longer stable identifier) as merge keys on arrays of tables. If you author a custom array-of-tables that should be replaceable-by-key rather than append-only, pick **one** convention (either `code` on every item, or `id` on every item) and stick with it across the whole array. Mixing `code` on some items and `id` on others falls back to append — the resolver won't guess which key to merge on.
|
||||||
|
|
||||||
### Some agent fields are read-only
|
### Some agent fields are read-only
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -21,9 +21,11 @@ no virtualenv — plain `python3` is sufficient.
|
||||||
Merge rules (purely structural — no field-name special-casing):
|
Merge rules (purely structural — no field-name special-casing):
|
||||||
- Scalars (string, int, bool, float): override wins
|
- Scalars (string, int, bool, float): override wins
|
||||||
- Tables: deep merge (recursively apply these rules)
|
- Tables: deep merge (recursively apply these rules)
|
||||||
- Arrays of tables where every item carries a `code` or `id` field:
|
- Arrays of tables where every item shares the *same* identifier
|
||||||
|
field (every item has `code`, or every item has `id`):
|
||||||
merge by that key (matching keys replace, new keys append)
|
merge by that key (matching keys replace, new keys append)
|
||||||
- All other arrays (scalars, mixed, or tables without code/id):
|
- All other arrays — including arrays where only some items have
|
||||||
|
`code` or `id`, or where items mix the two keys:
|
||||||
append (base items followed by override items)
|
append (base items followed by override items)
|
||||||
|
|
||||||
No removal mechanism — overrides cannot delete base items. To suppress
|
No removal mechanism — overrides cannot delete base items. To suppress
|
||||||
|
|
@ -92,7 +94,14 @@ def load_toml(file_path: Path, required: bool = False) -> dict:
|
||||||
|
|
||||||
|
|
||||||
def _detect_keyed_merge_field(items):
|
def _detect_keyed_merge_field(items):
|
||||||
"""Return 'code' or 'id' if every table item carries that field, else None."""
|
"""Return 'code' or 'id' if every table item carries that *same* field.
|
||||||
|
|
||||||
|
All items must share the same identifier (all `code`, or all `id`).
|
||||||
|
Mixed arrays — where some items use `code` and others use `id` —
|
||||||
|
return None and fall through to append semantics. This is intentional:
|
||||||
|
mixing identifier keys within one array is a schema smell, and
|
||||||
|
append-fallback is safer than guessing which key should merge.
|
||||||
|
"""
|
||||||
if not items or not all(isinstance(item, dict) for item in items):
|
if not items or not all(isinstance(item, dict) for item in items):
|
||||||
return None
|
return None
|
||||||
for candidate in _KEYED_MERGE_FIELDS:
|
for candidate in _KEYED_MERGE_FIELDS:
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue