Skip to content

Phase 10 plan — adventure-bundled items - #50

Merged
mmacy merged 3 commits into
mainfrom
phase-10-plan
Aug 6, 2026
Merged

Phase 10 plan — adventure-bundled items#50
mmacy merged 3 commits into
mainfrom
phase-10-plan

Conversation

@mmacy

@mmacy mmacy commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Implementation plan for phase 10 of the spec: an `Adventure` document carries its own item templates — the same four models the equipment catalog ships, discriminated on `item_type` — resolved everywhere the engine resolves authored item ids, mirroring the phase 9 bundled-monster contract. Written and reviewed per the phase loop: research → draft → rubber-duck → revise until SOLID.

Notable decisions

  • The mirror holds almost exactly. `Adventure.items`, an `_effective_equipment` helper beside `_effective_monsters`, the `effective_equipment` session property phase 9 explicitly reserved, built in `init` as the typed doctored-save backstop, first-occurrence-wins dedup filtered before construction, identity-on-empty-bundle. `validate_adventure` and `validate_content_pack` keep their exact signatures; osr-forge's calls are correct unchanged.
  • Three-way collision rule. Bundled ids must collide with neither the equipment catalog, the magic-item catalog, nor each other — the shipped namespaces are disjoint today (verified: 51 vs 164 ids, no overlap), phase 11's `has_item` matches "equipment or magic item" by one id, and the collision rule keeps that identity space single. `treasure_weights` is deliberately outside it (it already shares `"staff"` with the weapons list — an encumbrance table, not item identity), pinned by test.
  • The town-shop carve-out. `PurchaseEquipment` keeps resolving the shipped catalog: the spec's town offers "the equipment lists" (the SRD's commodity lists), purchase materializes a fresh instance (the cache idol must not have a second copy on a shelf), and `cost_gp` is a required field — without the carve-out every bundled quest object is automatically purchasable and phase 15's fetch quest could be bought closed. A bundled id at the shop refuses honestly with the new `items.purchase.not_stocked` code, distinct from `session.command.unknown_item` on the spec's own gate_refused-vs-locked posture.
  • A deletion, not a redirect, on the `GiveItems` path. The giver provably carries the instance, so `_grant_mundane` takes the giver's embedded template instead of re-resolving a catalog — closing a latent crash (`load_equipment().get` on a bundled id) before it is reachable.
  • No release work item. Phase 10's roadmap entry names no version; the changelog bullet rides `[Unreleased]` into the next minor.

Review provenance

The rubber-duck (a fresh reviewer with the spec, the phase 9 plan, and the touched code) returned NEEDS REVISION on one blocking finding — the crawl-site audit missed `views.py`'s armour-masking `base_item_id` lookup, which also invalidated the stated justification for redirecting the fixed-id ignite sites — plus six non-blocking refinements: the `_grant_mundane` template-passing shape, three docstring-sweep gaps (`EquipItem`, `validate_content_pack`, the guide's closed-loader paragraph), the honest rejection code, `create_character` precision (caller-supplied pre-session ids, not shipped-by-construction), naming phase 15's idol conversion as the deferred consumer, and the treasure-weights-exclusion test. All seven were adopted; the ignite redirects were dropped as churn. Re-review verified every fix against the code (including the template-capture safety property under repeated ids in one `GiveItems`) and returned SOLID, with two sign-off notes folded in: the `tools/docs/rejection_codes.json` drift-gate entry and two over-claiming sentences tightened.

https://claude.ai/code/session_01NSecUMBnHV9xSis9NSgPaF

@mmacy
mmacy merged commit 65e22a2 into main Aug 6, 2026
5 checks passed
@mmacy
mmacy deleted the phase-10-plan branch August 6, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant