[sandgraal]

Ticket

M14-07 — wowlab char macros, and the macro placement candidate rule. Branch m14/07-char-macros, head f66e528 (first push fe6e068, then review probe d18ecea, fix rounds f00aad4, 54fd434 and f66e528), cut from main 8b9c290.

Acceptance — with proof

• Text and --json on the fixture install, validating → pytest lab/core/tests/test_cli_char_macros.py (test_char_macros_on_the_fixture_install_with_no_record_places_nothing, test_char_macros_json_1_macro_none_2_findings_cross_capture, test_char_macros_text_on_the_fixture_install_cross_capture) → pass. The JSON round-trips through CharMacrosReport.model_validate_json; the install tree is unchanged after each run (L1).
• Unconfirmed with one candidate slot → test_unconfirmed_with_one_candidate_slot_and_a_key_constructed → pass. Character macro 29 gets index 149 and slot 64, the added constructed bind line CTRL-F7 reaches it; macro 20 gets 140 and slot 65; text shows "may sit at slot 64 (unconfirmed)".
• Unconfirmed with two candidate slots → test_unconfirmed_with_two_candidate_slots_constructed → pass (slots [64, 65]).
• None → fixture plus the real record: index 121, no slot holds it. Also: no record (the bare fixture), a schema-1 record, a constructed index past 120, a constructed slot with a subtype, constructed bars with a slot missing its type, a constructed absent section → pass.
• None on no bar → test_none_on_no_bar_with_the_checkers_finding_constructed → pass. Placement is none, and each macro carries the checker's own not-on-action-bar finding.
• Findings → test_findings_are_the_checkers_own_joined_by_macro_constructed → pass. Findings and per-macro cannot-judge entries, joined across all macros, equal macros check --json exactly.
• No output carries confirmed → test_no_output_carries_confirmed (6 setups: fixture-install, cross-capture, 4 constructed; x 3 character choices; text and JSON; regex (?<![A-Za-z])confirmed) → pass.
• macros list / macros check byte-identical to main → 10 invocations (list and check, text and JSON; account, character, schema-1 character) captured at origin/main 8b9c290 before editing and again after; diff -r before after → IDENTICAL, exit codes too.
• Runbook exists → docs/handoffs/M14-macro-place.md.
• make ci green (implementer's run, exit 0). Tail: 8613 passed, 208 skipped (test); 66 passed, 8824 deselected (parser); 1746 passed, 24 skipped; hooks 182 passed under 3.12 and 182 under 3.9; lint "All checks passed!", "464 files already formatted", mypy "Success: no issues found in 44 source files".
• Ticket acceptance commands run (above).
• luadata.py, guard.py or another parser changed? No. Graders activated: n/a.
• New external-format parsing? No new format; reuses the existing macro, bindings and record readers. Constructed inputs are labelled constructed (hostile and boundary cases).
• New runtime dependency? none
• User-facing surface:
  • New command wowlab char macros [--character] [--account] [--root] [--flavor] [--json]. It reads only and exits 0 even without a record. For each macro it prints the header line, the body lines, a placement: <status>: <reason> line, "may sit at slot n (unconfirmed): <bar/page>" lines with the keys that reach each slot, then finding <rule> (line n): <checker detail> and cannot judge lines. After that come "Cannot judge for any macro" and the notes.
  • --json is CharMacrosReport{flavor_folder, account, character, chosen_by, files[...], bindings_files[CharBindsFile], record_file, record_mtime_ns, record_schema, record_error, keys_incomplete (str | null), macros[{scope, file, position, line, id, name, icon, body_lines, complete, bytes, characters, placement{status, candidate_index, slots[{slot, page_note, unfollowed_page}], keys[keymap.KeyRow], reason}, findings[macrocheck.Finding], cannot_judge[macrocheck.CannotJudge]}], cannot_judge, notes}.
  • When the keys are incomplete (keys_incomplete set), the text qualifies "no key reaches it ... among the keys read ()" and prints every keymap note. The no-record wording matches macros check.
  • macros list and macros check are unchanged (byte-identical, above).
  • Wiki pages affected: char and macros.
  • Also touched: docs/GLOSSARY.md (entry "Macro id, macro index"), .github/release-notes.md, new library module lab/core/src/wowlab_core/macroplace.py, keymap.py (+11 lines, review round 1), and the merged review test lab/core/tests/review/test_m14_07_no_key_claim_without_bindings_record.py.
  • Review rounds renamed two tests with a _constructed suffix (test_a_schema_1_record_places_nothing_constructed, test_char_macros_without_character_takes_the_latest_record_cross_capture_constructed); in the proof lines above index now reads candidate_index in the JSON.

new runtime dependencies: none

Invariants (AGENTS.md)

• L1 reads never write: command is read-only; install tree compared unchanged after each run in the tests
• L2 one write gate: no write site added
• L3 n/a: no Lua parsing added
• L4 n/a: no parser added or changed
• L5 n/a: no game data touched
• L6 ACCOUNT_MACROS = 120 is a client UI constant, marked candidate + [verify] everywhere in output; ruled acceptable under L6 by the domain-reviewer. No flavor, product, interface or build constant; the library discovers the flavor folder
• L7 nothing ADR-0023 excludes: placement is inferred from files on disk, no process memory, injection or input automation
• L8 real fixtures where they exist; constructed inputs are labelled _constructed and limited to boundary and hostile cases; no live calls

Judgment calls

Claims for the reviewers to test:

• Candidate rule numbering: an account-macro record k gets index k for k <= 120; a character-macro record k gets index 120 + k; file order; marked [verify] everywhere in output. Source: LAB_FORMATS 2026-10-16 item 7. Placement is never reported as confirmed; the best status is unconfirmed.
• "No subtype" is read as a falsy sub_type on an action-bar slot.
• Report fields beyond the LAB_PLAN section 16.2 sketch follow the CharBindsReport style.

Reviews

Reviewed by: domain-reviewer FINDINGS(9) at fe6e068 (file-order attribution, runbook mtimes/cap/example, candidate_index rename, stale-cache note; none wrong), all fixed at f00aad4, one runbook polish fixed at 54fd434, CLEAN. code-reviewer FINDINGS(2)+1 at fe6e068 (every keymap note carried, test labels, no-record wording), CLEAN at f00aad4 (758 passed over the char/keymap/review suites, make lint clean). Round 1 make ci: 8619 passed; round 2 is docs-only. Review job FINDINGS(1), runbook numbering → fixed f66e528, graded CLEAN by code-reviewer. Head f66e528.

---

session: 284d1153-c891-4b84-8e47-150842fac4ba
🤖 Generated with Claude Code

[claude]

Claude encountered an error after 2m 47s —— View job

---

Independent review (code-reviewer): M14-07 wowlab char macros

• Read AGENTS.md, ticket M14-07, LAB_PLAN §16.2, LAB_FORMATS 2026-10-16 item 7
• Read the diff (.pr-review/pr.diff, all 8 files)
• Checked CI: still running when this review finished, so there is no result yet
• Checked invariants L1–L8 and the user-facing surface
• Posted findings and verdict

What should be true (from the spec, before reading the diff)

• A new read-only leaf char macros with the four options char show declares, plus --json.
• It covers every record of the account and character macros-cache.txt, read the way macros list and macros check read them.
• Placement is unconfirmed (by the candidate rule: macro index when the slot has no subtype), or none. confirmed is in the model, but nothing emits it.
• Findings are macrocheck's, called and not copied, joined by macro.
• macros list and macros check are unchanged.
• The runbook is docs/handoffs/M14-macro-place.md.

Findings

1. Low, docs. docs/handoffs/M14-macro-place.md:96: section 1's list renders with different numbers than the text uses. The source numbers jump from 3 to 5. GitHub renumbers the list, so the /dump step renders as step 4 and the drag step as step 5. The §5 report table and lines 199 and 211 still call the /dump step "step 1.5", so the owner is pointed at the wrong step when filling in the table. Details are in the inline comment.

Checked, no finding

• L1. char macros opens nothing for writing. It uses snapshot.read_regular_file with _MACROS_LIMIT (as macros check does), wtfconfig.read_bindings, and the existing _coach_record. macroplace.py opens no file at all.
• L2, L3, L4, L5. The branch adds no write site, no Lua handling, no parser change and no game-data cache.
• L6. ACCOUNT_MACROS = 120 is a client UI constant, not a flavor, product, interface or build constant. It is marked [verify] in the docstring, in CANDIDATE_RULE and in every reason string. Nothing in macroplace.py or the new CLI code names a flavor folder. The runbook's _classic_beta_ is a doc example, and it tells the owner to substitute their own folder name.
• L7. Placement is inferred from files on disk only.
• L8. Constructed cases carry _constructed in the test id. The real-record case is test_place_on_the_real_record_gives_two_unconfirmed.
• Same reading as macros check. The file lookup is the same _wtf_file. _select_character accepts the realm/char form that who passes. The no_record and no_book wording matches _check_record's. macrocheck.check returns one CheckedMacro per source, in order, so zip(..., strict=True) holds. Findings are joined on MacroRef(scope, id, line), the same key macrocheck._ref builds. Cannot-judge entries for the whole report (macro is None) are kept at report level. That includes the "at least N on no bar" count, which macrocheck._check_bars puts in the reason text, so nothing that MacrosCheckReport.on_no_bar_at_least carries is lost.
• "No subtype" check. macroplace tests sub_type for falsiness, while macrocheck tests is None. Both behave the same here: labaddon.ActionWord requires ^[A-Za-z]{1,32}$, so an empty string cannot occur.
• keymap.KeyMap.keys_incomplete. The new field is required, and KeyMap has one constructor (keymap.py:561). KeyMap is never emitted directly: CharBindsReport copies chosen fields and does not gain this one. So char binds --json and binds coach --json are unchanged.
• confirmed. test_no_output_carries_confirmed covers 6 setups across text and JSON, with a word-boundary regex. Placement.status allows confirmed, and no code path builds it.
• User-facing surface. The new command, the CharMacrosReport shape, the text lines, the keys_incomplete wording, the release notes, the GLOSSARY entry and the wiki pages (char, macros) are all in the PR body. I found no unlisted change.
• Other checks. No new runtime dependency. Nothing from LAB_IDEAS. No personal identifiers in the new tests. No workflow changes.

What I read

• AGENTS.md, .claude/agents/code-reviewer.md, BACKLOG M14-07, and LAB_PLAN §16.2.
• .pr-review/pr.diff: release notes, GLOSSARY and the cli.py hunks.
• In full: macroplace.py and the runbook.
• keymap.py hunks, and the macrocheck.py sections for MacroRef, CheckResult, _check_bars and check.
• The cli.py helpers _wtf_file, _select_character, _check_record, macros_check and char_binds.
• The test ids in test_cli_char_macros.py.

I ran no code, as the instructions require. CI (CI and Windows) was still in progress, so its result is not part of this verdict, and a red required check would be a finding.

verdict: FINDINGS(1)