Skip to content

Commit 95621cc

Browse files
pdbethkeclaude
andcommitted
Merge: link skills to their characteristic, and make the canonical path the default
A skill names its characteristic only inside <CHARACTERISTIC_CHOICE>. The HDT provider read that block for its costs and dropped the name, so a document that omits CHARACTERISTIC produced a general-level roll — a flat 11 — instead of 9 + characteristic/5 (6E1 p57). That path is kirby-api's relational re-emission and the in-app editor; every HD-saved .hdc states the characteristic, so the corpus never saw it. Also bundles the three authored characters and retires the env-var gate that kept the canonical loader out of the default run. Fourteen tests were skipping on one unset variable while sixteen dump-based tests stood in for them — and the dump is lossy on exactly the two things this change touches, which cost us two false alarms before anyone loaded the real file. Oracle parity held at 656/656 with the residual ledger untouched. Suite goes 1732 passed / 14 skipped to 1753 passed / 0 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PrVDXgpDfQpzEGErZTLjEs
2 parents a555913 + 78c5abb commit 95621cc

17 files changed

Lines changed: 1223 additions & 41 deletions

.env.test.example

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ KIRBY_COST_HD6CLI=~/path/to/hd6cli.sh
4343
# artifact to name and no default worth writing.
4444
KIRBY_COST_ROUNDTRIP_HDC=~/path/to/a-complex-character.hdc
4545

46-
# A directory holding the three authored characters cleared for use as
47-
# corpus, by name: Ravel.hdc, Bokor.hdc, PowerLad.hdc. Symlinks are fine —
48-
# they need not live together on disk. The .hdc files are NOT redistributed;
49-
# only the oracle's JSON dumps of them ship, and those carry costs and
50-
# display strings with no campaign material in them. Unset: the three
51-
# authored-character tests skip.
46+
# OPTIONAL, and an override rather than a requirement since 2026-09-01: all
47+
# three authored characters (Ravel.hdc, Bokor.hdc, PowerLad.hdc) now ship in
48+
# tests/fixtures/authored/, so nothing skips when this is unset. Set it to a
49+
# directory holding those three names to run against your own working copies
50+
# instead — symlinks are fine, they need not live together on disk. A name
51+
# this variable does not resolve falls back to the bundled copy.
5252
#KIRBY_COST_AUTHORED=/path/to/authored

.gitignore

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,27 @@ legal/
7272
# roundtrip_hd6_costs.json is one character's costed build. Same rule.
7373
tests/fixtures/oracle/
7474
tests/fixtures/*.hdc
75+
tests/fixtures/**/*.hdc
7576
tests/fixtures/roundtrip_hd6_costs.json
77+
78+
# DELIBERATELY TRACKED, and the only .hdc files in the repository:
79+
# tests/fixtures/authored/{Ravel,Bokor,PowerLad}.hdc are PeterB's own
80+
# characters, built on stock templates and cleared by him for publication
81+
# (2026-09-01). They are here so the canonical load path -- HDCLoader on a
82+
# real HD-saved file -- runs by default instead of only where an environment
83+
# variable happened to be set.
84+
#
85+
# The `tests/fixtures/**/*.hdc` line above is what makes these three
86+
# negations load-bearing, and it was added for that reason. `*.hdc` alone
87+
# does not match a subdirectory, so before it every .hdc under
88+
# tests/fixtures/authored/ was tracked by default -- including any licensed
89+
# third-party character dropped there, which `git add -A` would have staged
90+
# without a word. Deny everything under tests/fixtures/, re-admit exactly
91+
# three by name.
92+
#
93+
# THREE FILES, NO MORE. Nothing else from that document store may be added:
94+
# the rest is licensed third-party content and personal campaign material,
95+
# including the variants that sit beside these (background sheets, kits).
96+
!tests/fixtures/authored/Ravel.hdc
97+
!tests/fixtures/authored/Bokor.hdc
98+
!tests/fixtures/authored/PowerLad.hdc
Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
# Canonical fixture correction — `fix/skill-characteristic-linkage`
2+
3+
*2026-09-01*
4+
5+
## The question
6+
7+
"Does not canonical imply default?" It did not. `HDCLoader` is this project's
8+
canon — 100% Java-oracle parity, and the only path that reads what HERO
9+
Designer actually wrote. But the skill-roll tests on this branch asserted
10+
against `tests/fixtures/authored/Ravel.json`, an **oracle dump**, and the one
11+
test that loaded through the canonical path was gated on `KIRBY_COST_AUTHORED`,
12+
which nothing set. Sixteen dump-based tests ran; the faithful one did not.
13+
14+
The dump is lossy in two ways that matter:
15+
16+
1. It omits `CHARACTERISTIC` on every skill.
17+
2. It does not record which ITEM a multi-ITEM skill (`PROFESSIONAL_SKILL`,
18+
`SCIENCE_SKILL`) was bought as, so the engine can only take the template's
19+
first.
20+
21+
That produced two false alarms in one evening — five "proficiency regressions"
22+
and two "multi-ITEM build-doc gaps" — both of which evaporate on a real load.
23+
24+
## What changed
25+
26+
**1. The characters are bundled.** `tests/fixtures/authored/` now holds three
27+
`.hdc` files, verified by `CHARACTER_NAME` before copying:
28+
29+
| file | bytes | `CHARACTER_NAME` |
30+
|---|---|---|
31+
| `Ravel.hdc` | 172,400 | `Ravel, The Unmade Man` |
32+
| `Bokor.hdc` | 140,178 | `Bokor` |
33+
| `PowerLad.hdc` | 85,944 | `Power Lad` |
34+
35+
md5 verified identical to the originals. **Nothing else was copied** from
36+
`~/Documents/Champions` — not `Ravel_background.hdc`, not `Ravel (CSI Kit).hdc`,
37+
not any licensed third-party character. `.gitignore` gained explicit
38+
`!` negations for exactly these three plus a comment saying three files, no
39+
more, so tightening `tests/fixtures/*.hdc` later cannot silently drop them and
40+
a fourth cannot be added by accident.
41+
42+
**2. `authored_hdc()` finds them with no environment variable.**
43+
`tests/corpus.py` gained `BUNDLED_AUTHORED`; `authored_hdc(name)` checks
44+
`KIRBY_COST_AUTHORED` first and falls back to the bundle. The override is
45+
per-name, so a maintainer pointing at his own store still gets his working
46+
copies, and a name that store lacks still resolves.
47+
48+
**3. A missing canonical fixture FAILS.** New `require_authored_hdc(name)`
49+
raises `FileNotFoundError` naming the bundled path. The canonical tests carry
50+
**no skip guard at all** — in `test_skill_characteristic_roll.py`,
51+
`test_campaign_cost_fields.py`, and `test_authored_characters.py`'s `_cases()`.
52+
These files are tracked, so absence is a deletion, not an unconfigured machine.
53+
54+
**4. The skip guard was ARMED, not bypassed.** `conftest.py`'s design is
55+
untouched: it still reads `INPUTS` twice, to report what a run was configured
56+
with and to decide whether a skip is acceptable or a defect.
57+
`KIRBY_COST_AUTHORED` stays in `INPUTS` — it is still honoured as an override.
58+
What changed is `missing_inputs()`: via `_BUNDLE_SATISFIES`, an input the
59+
repository itself carries is no longer "missing". That tightens the guard
60+
rather than loosening it. The run header on this machine now reads:
61+
62+
```
63+
kirby-cost: all 9 inputs present — any skip will fail the run
64+
```
65+
66+
Before, it reported `KIRBY_COST_AUTHORED` missing and the guard excused all 14
67+
skips. Three tests in `test_test_inputs.py` pin the new contract (bundle
68+
satisfies; variable still overrides; missing bundle raises).
69+
70+
**5. The tests moved to the canonical path.** `test_skill_characteristic_roll.py`
71+
is now explicitly two-lane, and says which lane each test is in:
72+
73+
- **Canonical** — the `ravel` fixture is `HDCLoader().load_file(...)`,
74+
function-scoped and reloaded per test on purpose, because `load_file`
75+
*installs the active hero* and `roll_value`'s characteristic branch reads it.
76+
Caching it would be correct only until another test loaded another character.
77+
- **Degraded, deliberately** — the `ravel_dump` fixture builds from the JSON
78+
dump, and is the only shape in which the template fallback fires at all
79+
(across the 655-character corpus it fires on zero of 4,434 skill-like
80+
objects). `test_the_dump_states_no_characteristic_at_all` pins that premise,
81+
so the fallback tests cannot go on passing while testing nothing. The
82+
familiarity/Everyman short-circuit tests keep building from a mutated doc,
83+
labelled as such.
84+
85+
**6. The test that would have caught tonight.**
86+
`test_ravel_reproduces_every_roll_hero_designer_printed` loads Ravel
87+
canonically and asserts sixteen skill rolls against HD's own rendered output.
88+
`test_the_transcribed_rolls_are_still_the_ones_hero_designer_printed`
89+
re-derives that table from the dump's `column2_output` so the transcription
90+
cannot rot into a test of a typo. Seven of the sixteen — the five Proficiencies
91+
and both multi-ITEM skills — are precisely the cases a dump-based test cannot
92+
state.
93+
94+
## Ravel against HD's printed rolls, via the canonical loader
95+
96+
Every skill for which HD renders a roll. **16 of 16 agree, 0 mismatched.**
97+
98+
| skill | characteristic | engine | HD printed |
99+
|---|---|---|---|
100+
| PROFESSIONAL_SKILL | INT | 14 | 14 |
101+
| SCIENCE_SKILL | INT | 14 | 14 |
102+
| DEDUCTION | INT | 14 | 14 |
103+
| CRIMINOLOGY | INT | 14 | 14 |
104+
| FORENSIC_MEDICINE | INT | 14 | 14 |
105+
| PARAMEDICS | INT | 14 | 14 |
106+
| COMPUTER_PROGRAMMING | INT | 14 | 14 |
107+
| NAVIGATION | INT | 14 | 14 |
108+
| ACTING | PRE | 13 | 13 |
109+
| CHARM | PRE | 13 | 13 |
110+
| KNOWLEDGE_SKILL | GENERAL | 13 | 13 |
111+
| HIGH_SOCIETY | PRE | 10 | 10 |
112+
| BUREAUCRATICS | PRE | 10 | 10 |
113+
| SECURITY_SYSTEMS | INT | 10 | 10 |
114+
| STREETWISE | PRE | 10 | 10 |
115+
| INTERROGATION | PRE | 10 | 10 |
116+
117+
`LANGUAGES` and `SKILL_LEVELS` are excluded because HD renders no roll for
118+
either, and the provenance test asserts that exclusion set exactly.
119+
120+
The five Proficiencies sit at 10 despite PRE/INT linkage: `roll_value`
121+
short-circuits on the proficiency flag before it consults the characteristic
122+
(6E1 p57 sets the linked base; a Proficiency is bought as a flat roll).
123+
124+
## Gates
125+
126+
**Oracle (release gate):** `656 passed`, exit 0.
127+
`tests/fixtures/oracle_known_residuals.json` untouched — `git status` reports
128+
no modification to it.
129+
130+
**Full suite**, `KIRBY_COST_HDT` pointed at the complete 17-template directory:
131+
132+
| | passed | skipped |
133+
|---|---|---|
134+
| before (stashed working tree) | 1732 | **14** |
135+
| after | **1753** | **0** |
136+
137+
The 14 skips were all `KIRBY_COST_AUTHORED unset`: 12 in
138+
`test_authored_characters.py` (4 parametrized tests x 3 characters), 1 in
139+
`test_campaign_cost_fields.py`, 1 in `test_skill_characteristic_roll.py`.
140+
All 14 now run. **Zero skips remain.**
141+
142+
+21 net passing: 12 previously-skipped authored tests + 2 previously-skipped
143+
canonical tests, plus the new canonical and provenance tests in
144+
`test_skill_characteristic_roll.py` and three new guard tests in
145+
`test_test_inputs.py`, less the dump-based tests that were folded into the
146+
canonical ones.
147+
148+
## The caution that did not fire
149+
150+
`test_authored_characters.py` compares each `.hdc` against its JSON fixture on
151+
every cost and every display string, for all three characters. Those twelve
152+
tests had never run in this environment. **They pass on first run** — no
153+
disagreement between the real files and the dumps on anything those tests
154+
compare. That is a genuinely useful negative result: the dumps are faithful on
155+
*costs and strings*, and lossy specifically on the **skill characteristic
156+
linkage and multi-ITEM selection**, which is the narrow seam this branch is
157+
about. Nothing was adjusted to make anything pass.
158+
159+
## Not touched
160+
161+
`Hero.characteristic_value()` — unchanged.
162+
`tests/fixtures/oracle_known_residuals.json` — unchanged.
163+
`conftest.py` — unchanged; the guard mechanism it implements is now armed by
164+
default rather than modified.
165+
166+
---
167+
168+
# Fix round — the licensing rail was inert
169+
170+
*2026-09-01, after review*
171+
172+
## What was wrong
173+
174+
The three `!tests/fixtures/authored/*.hdc` negations I added were **inert**,
175+
and worse than inert: they were a claim of protection where none existed.
176+
177+
The pre-existing pattern is `tests/fixtures/*.hdc`. A single `*` does not
178+
cross a directory separator, so that line never reached
179+
`tests/fixtures/authored/` at all. Nothing there was ever ignored, so
180+
negating it re-admitted nothing. The three bundled files stayed tracked for
181+
the same reason they would have without any of my lines: they were never
182+
matched.
183+
184+
The consequence is the part that mattered. A licensed third-party `.hdc`
185+
dropped into `tests/fixtures/authored/` would have been picked up by
186+
`git add -A` with no warning — the exact failure the comment above it claimed
187+
to prevent. My report said a fourth file "cannot be added by accident" and the
188+
SKILL.md said the three were "explicitly un-ignored". Both described a
189+
mechanism that was not running. A false assurance on the rail that keeps
190+
licensed content out of a public repo is worse than no claim at all, because
191+
it stops anyone from checking.
192+
193+
## The fix
194+
195+
One line, above the negations, which is what makes them load-bearing:
196+
197+
```
198+
tests/fixtures/oracle/
199+
tests/fixtures/*.hdc
200+
tests/fixtures/**/*.hdc <-- added
201+
tests/fixtures/roundtrip_hd6_costs.json
202+
...
203+
!tests/fixtures/authored/Ravel.hdc
204+
!tests/fixtures/authored/Bokor.hdc
205+
!tests/fixtures/authored/PowerLad.hdc
206+
```
207+
208+
Deny every `.hdc` anywhere under `tests/fixtures/`, then re-admit exactly
209+
three by name. The surrounding comment now says *why* the `**` line exists and
210+
tells the next reader to verify with `git check-ignore -v` rather than trust
211+
the prose — since trusting the prose is what went wrong here.
212+
213+
## Proof, run after the change
214+
215+
```
216+
$ git check-ignore -v "tests/fixtures/authored/Ravel_background.hdc"
217+
.gitignore:75:tests/fixtures/**/*.hdc tests/fixtures/authored/Ravel_background.hdc
218+
exit=0
219+
220+
$ git check-ignore -v "tests/fixtures/authored/Ravel (CSI Kit).hdc"
221+
.gitignore:75:tests/fixtures/**/*.hdc tests/fixtures/authored/Ravel (CSI Kit).hdc
222+
exit=0
223+
224+
$ git ls-files tests/fixtures/ | grep hdc
225+
tests/fixtures/authored/Bokor.hdc
226+
tests/fixtures/authored/PowerLad.hdc
227+
tests/fixtures/authored/Ravel.hdc
228+
229+
$ git status --porcelain
230+
M .gitignore
231+
M .superpowers/canonical-fixture-report.md
232+
M tests/test_skill_characteristic_roll.py
233+
```
234+
235+
Both licensed names are now ignored, and `.gitignore:75` names the line doing
236+
it. Exactly the three bundled files remain tracked. The porcelain output shows
237+
only this fix round's own edits and no stray `.hdc`.
238+
239+
**Live drill, not just a dry check.** I copied the real
240+
`Ravel_background.hdc` into `tests/fixtures/authored/`, ran `git add -A .`,
241+
and confirmed the staged set:
242+
243+
```
244+
$ git diff --cached --name-only | grep hdc
245+
NONE STAGED
246+
```
247+
248+
The file was physically present in the directory and `git add -A` did not
249+
stage it. Removed afterwards; the directory holds the three bundled `.hdc`
250+
and their four `.json` fixtures again.
251+
252+
## Minors
253+
254+
- "fifteen skill rolls" corrected to **sixteen** in both places in this
255+
report. `HD_PRINTED_ROLLS` has sixteen entries and the table above lists
256+
sixteen rows; the prose was simply wrong.
257+
- `test_the_transcribed_rolls_are_still_the_ones_hero_designer_printed` no
258+
longer takes the `ravel` fixture. It reads HD's rendered output and the
259+
transcription and never loads a character, so the parameter was buying an
260+
`HDCLoader` run per invocation for nothing. Its docstring now says it takes
261+
no hero, so the parameter does not come back.
262+
- SKILL.md's "explicitly un-ignored" wording replaced. It now states what is
263+
actually true — everything under `tests/fixtures/` is denied, exactly three
264+
are negated back in — names the `**` line as the one that reaches
265+
subdirectories, and records that negations against `*.hdc` alone were inert,
266+
so the next person does not repeat this.
267+
268+
## Gates re-run after the fix
269+
270+
| | result |
271+
|---|---|
272+
| Oracle | **656 passed**, exit 0; `oracle_known_residuals.json` unmodified |
273+
| Full suite | **1753 passed, 0 skipped**, exit 0 |
274+
275+
Unchanged from before the fix round, as expected: nothing here touches engine
276+
behaviour, and dropping an unused fixture parameter removes a redundant load
277+
rather than a test.

0 commit comments

Comments
 (0)