Merge pull request #62 from ViPro-Technologies/NEO-33-e2m1-lock-prototype-skilldef-trio-ci-catalog-contract
NEO-33: Lock prototype SkillDef trio + CI catalog contractpull/63/head
commit
7b307e503c
|
|
@ -7,6 +7,8 @@ Data-driven definitions (skills, items, recipes, quests) validated in CI with **
|
|||
| [`schemas/`](schemas/) | JSON Schema files (`*.schema.json`) |
|
||||
| [`skills/`](skills/) | Skill catalogs; each row matches [`schemas/skill-def.schema.json`](schemas/skill-def.schema.json) — **stable `id`**, **`allowedXpSourceKinds`** for [E2.M1](../docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md) / [E2.M2](../docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md) |
|
||||
|
||||
**Prototype Slice 1 (NEO-33):** CI expects **exactly three** skill rows with ids **`salvage`**, **`refine`**, **`intrusion`** (gather + process + tech). Each row must declare **`allowedXpSourceKinds`** (non-empty); see [E2.M1 — Designer note: `allowedXpSourceKinds`](../docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md#designer-note-allowedxpsourcekinds-and-grant-call-sites) for what each kind means for grant wiring.
|
||||
|
||||
Server load path and hot-reload policy are TBD; keep files versioned here as the source of truth.
|
||||
|
||||
**Validate locally** (same as PR gate):
|
||||
|
|
|
|||
|
|
@ -41,7 +41,7 @@ Content tooling **Slice 1** — schemas + CI; **Slice 4** — server parity hard
|
|||
|
||||
## CI (prototype)
|
||||
|
||||
**Skill catalogs:** the repo **PR gate** workflow ([`.github/workflows/pr-gate.yml`](../../../.github/workflows/pr-gate.yml)) runs [`scripts/validate_content.py`](../../../scripts/validate_content.py) on every push and pull request. It validates each object in `content/skills/*.json` under the top-level `skills` array against [`content/schemas/skill-def.schema.json`](../../../content/schemas/skill-def.schema.json) (Python **jsonschema**, Draft 2020-12). Extend the script when additional catalogs and schemas ship.
|
||||
**Skill catalogs:** the repo **PR gate** workflow ([`.github/workflows/pr-gate.yml`](../../../.github/workflows/pr-gate.yml)) runs [`scripts/validate_content.py`](../../../scripts/validate_content.py) on every push and pull request. It validates each object in `content/skills/*.json` under the top-level `skills` array against [`content/schemas/skill-def.schema.json`](../../../content/schemas/skill-def.schema.json) (Python **jsonschema**, Draft 2020-12), rejects **duplicate `id`** across files, and (Slice 1 / [NEO-33](https://linear.app/neon-sprawl/issue/NEO-33)) enforces the **frozen prototype trio** ids plus **gather + tech + (process or make)** category coverage. Extend the script when additional catalogs and schemas ship.
|
||||
|
||||
**Decomposition vocabulary:** the same workflow runs [`scripts/check_decomposition_language.py`](../../../scripts/check_decomposition_language.py) (stdlib only) over `docs/decomposition/**/*.md` to catch wording that treats **combat** as a leveled **`SkillDef`** line instead of **gig** progression (see script docstring for patterns). Adjust the script if a future doc needs a documented exception.
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
| **Epic** | [Epic 2 — Skills and Progression Framework](../epics/epic_02_skills_and_progression.md) |
|
||||
| **Linear (Epic 2 project)** | [Epic 2 — Classless Skill and Progression Framework](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6) |
|
||||
| **Stage target** | Prototype |
|
||||
| **Status** | Planned (see [dependency register](module_dependency_register.md)) |
|
||||
| **Status** | In Progress (see [dependency register](module_dependency_register.md)) |
|
||||
|
||||
## Linear backlog (Epic 2 Slice 1)
|
||||
|
||||
|
|
@ -54,6 +54,28 @@ Central catalog for **non-combat** skill metadata: **stable IDs**, **categories*
|
|||
|
||||
Add new enum values only with a **schema version** / migration note in [CT.M1](CT_M1_ContentValidationPipeline.md) when CI enforces catalogs.
|
||||
|
||||
### Designer note: allowedXpSourceKinds and grant call sites
|
||||
|
||||
Every `SkillDef` row **must** include a **non-empty** `allowedXpSourceKinds` array (enforced by JSON Schema). Designers use it to agree with engineering **which systems may ever grant XP** to that skill:
|
||||
|
||||
- **`activity`** — wire gather nodes, recipe completions, skill-based world objectives, and similar **primary-loop** payouts; set `XpGrantEvent.sourceKind` (or successor) to `activity` on those code paths.
|
||||
- **`mission_reward`** — wire **explicit** mission/contract hand-ins or scripted bonuses only; not a stand-in for combat encounter XP ([`docs/game-design/progression.md`](../../game-design/progression.md) — mission vs combat).
|
||||
- **`trainer`** / **`book_or_item`** — wire NPC teaching beats and consumable/one-shot grants respectively; do not use for combat kills.
|
||||
|
||||
**Combat encounters award gig XP, not skill XP.** If a grant channel is missing from a skill’s list, [E2.M2](E2_M2_XpAwardAndLevelEngine.md) must reject the grant once implemented—keep catalogs honest so call sites do not drift.
|
||||
|
||||
## Prototype Slice 1 freeze ([NEO-33](https://linear.app/neon-sprawl/issue/NEO-33))
|
||||
|
||||
The **first shipped trio** under `content/skills/*.json` is **frozen** for downstream references (quests, recipes, E2.M2, E3) until a deliberate migration or follow-up issue expands the roster.
|
||||
|
||||
| `SkillDef.id` | `category` | Role |
|
||||
|---------------|------------|------|
|
||||
| **`salvage`** | `gather` | Prototype Gather anchor |
|
||||
| **`refine`** | `process` | Prototype Process anchor (satisfies “Process or Make” for Slice 1) |
|
||||
| **`intrusion`** | `tech` | Prototype Tech anchor |
|
||||
|
||||
**Rules:** Do **not** rename these `id` values to “fix” wording—change `displayName` only, or add a new `id` and migrate content. CI ([`scripts/validate_content.py`](../../../scripts/validate_content.py)) enforces **exactly** these three ids across all skill JSON files and **category coverage** (gather + tech + at least one of process/make). Relaxing or changing that gate belongs in a new Linear issue once the catalog intentionally grows.
|
||||
|
||||
## Responsibilities
|
||||
|
||||
- Load and validate `SkillDef` from data; expose `SkillCategory` and `UnlockRequirement` for gating.
|
||||
|
|
@ -71,7 +93,7 @@ Add new enum values only with a **schema version** / migration note in [CT.M1](C
|
|||
|
||||
- Every loaded `SkillDef` validates against [`skill-def.schema.json`](../../../content/schemas/skill-def.schema.json) in CI ([CT.M1](CT_M1_ContentValidationPipeline.md#ci-prototype) — [`scripts/validate_content.py`](../../../scripts/validate_content.py) in **PR gate**) or at server boot.
|
||||
- **Duplicate `id`** in a catalog fails validation.
|
||||
- **Roster freeze for prototype:** when Slice 1 locks three skills, their **`id`** values are treated as **stable**—downstream content (recipes, quests) may reference them; changing an `id` requires a **migration** or new row.
|
||||
- **Roster freeze for prototype:** Slice 1 locks **`salvage`**, **`refine`**, and **`intrusion`**; their **`id`** values are **stable**—downstream content (recipes, quests) may reference them; changing an `id` requires a **migration** or new row.
|
||||
- Document for designers: each new skill row **must** declare **`allowedXpSourceKinds`** so engineers know which systems may grant XP (gather/craft hooks vs E7.M2 mission rewards only, etc.).
|
||||
|
||||
## Module dependencies
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not
|
|||
| E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) |
|
||||
| E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **NEO-27 landed:** Slice 3 telemetry hook sites documented — `selection_event` maps to product `target_changed` in `target_selection_client.gd`; server lock transition hook comments in `InMemoryPlayerTargetLockStore`; reserved `ability_cast_requested` / `ability_cast_denied` comments in `client/scripts/main.gd` (all TODO(E9.M1)); see [NEO-27](../../plans/NEO-27-implementation-plan.md) and [`docs/manual-qa/NEO-27.md`](../../manual-qa/NEO-27.md). **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`; production telemetry ingest/catalog after E9.M1. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) |
|
||||
| E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **NEO-32 landed:** `GET /game/players/{id}/cooldown-snapshot` + `on_cooldown` cast deny + prototype global cooldown commit; [`cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`cooldown_state.gd`](../../../client/scripts/cooldown_state.gd), **`CooldownSlotsLabel`** in [`main.gd`](../../../client/scripts/main.gd); manual QA [NEO-32](../../manual-qa/NEO-32.md); plan [NEO-32](../../plans/NEO-32-implementation-plan.md). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md), [NEO-32](../../plans/NEO-32-implementation-plan.md); [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md); [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md); `server/NeonSprawl.Server/Game/AbilityInput/`; [`client/scripts/hotbar_loadout_client.gd`](../../../client/scripts/hotbar_loadout_client.gd), [`client/scripts/hotbar_state.gd`](../../../client/scripts/hotbar_state.gd), [`client/scripts/ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd), [`client/scripts/hotbar_cast_slot_resolver.gd`](../../../client/scripts/hotbar_cast_slot_resolver.gd), [`client/scripts/cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`client/scripts/cooldown_state.gd`](../../../client/scripts/cooldown_state.gd); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31), [Cooldown snapshot (NEO-32)](../../../server/README.md#cooldown-snapshot-neo-32) |
|
||||
| E2.M1 | Planned | **Linear backlog (Todo):** [NEO-33](https://linear.app/neon-sprawl/issue/NEO-33) → [NEO-36](https://linear.app/neon-sprawl/issue/NEO-36) under [Epic 2 project](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6); label **`E2.M1`**. `content/schemas/skill-def.schema.json`, `content/skills/prototype_skills.json`, and PR-gate `validate_content.py` already exist; **server registry, fail-fast boot load, and read HTTP + Bruno** are not implemented yet. | [E2_M1_SkillDefinitionRegistry.md](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note** |
|
||||
| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **Follow-on:** server load ([NEO-34](https://linear.app/neon-sprawl/issue/NEO-34)), registry service ([NEO-35](https://linear.app/neon-sprawl/issue/NEO-35)), read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note** |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -27,12 +27,12 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
|
|||
|
||||
| Module ID | Module Name | Depends On | Contract Needed | Phase Required | Status |
|
||||
|---|---|---|---|---|---|
|
||||
| E2.M1 | SkillDefinitionRegistry | None | SkillDef, SkillCategory, UnlockRequirement, allowedXpSourceKinds | Prototype | Planned |
|
||||
| E2.M1 | SkillDefinitionRegistry | None | SkillDef, SkillCategory, UnlockRequirement, allowedXpSourceKinds | Prototype | In Progress |
|
||||
| E2.M2 | XpAwardAndLevelEngine | E2.M1 | XpGrantEvent, LevelCurve, LevelUpEvent | Prototype | Planned |
|
||||
| E2.M3 | MasteryAndPerkUnlocks | E2.M2 | MasteryTrack, PerkUnlockEvent, PerkState | Pre-production | Planned |
|
||||
| E2.M4 | ProgressionPacingControls | E2.M2, E9.M2 | XpModifierProfile, CatchUpRule, PacingPolicy | Pre-production | Planned |
|
||||
|
||||
**E2.M1 note:** Epic 2 Slice 1 backlog in Linear ([Epic 2 — Classless Skill and Progression Framework](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6)): [NEO-33](https://linear.app/neon-sprawl/issue/NEO-33) → [NEO-34](https://linear.app/neon-sprawl/issue/NEO-34) → [NEO-35](https://linear.app/neon-sprawl/issue/NEO-35) → [NEO-36](https://linear.app/neon-sprawl/issue/NEO-36); label **`E2.M1`**. See [E2_M1_SkillDefinitionRegistry.md](E2_M1_SkillDefinitionRegistry.md). Register stays **Planned** until implementation merges; update [documentation and implementation alignment](documentation_and_implementation_alignment.md) when code lands.
|
||||
**E2.M1 note:** Epic 2 Slice 1 backlog in Linear ([Epic 2 — Classless Skill and Progression Framework](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6)): [NEO-33](https://linear.app/neon-sprawl/issue/NEO-33) → [NEO-34](https://linear.app/neon-sprawl/issue/NEO-34) → [NEO-35](https://linear.app/neon-sprawl/issue/NEO-35) → [NEO-36](https://linear.app/neon-sprawl/issue/NEO-36); label **`E2.M1`**. See [E2_M1_SkillDefinitionRegistry.md](E2_M1_SkillDefinitionRegistry.md). **NEO-33** (content + CI + docs) moves the register row to **In Progress**; **NEO-34+** cover server registry and HTTP. Update [documentation and implementation alignment](documentation_and_implementation_alignment.md) when slices land.
|
||||
|
||||
### Epic 3 — Crafting Economy
|
||||
|
||||
|
|
|
|||
|
|
@ -126,7 +126,7 @@ Layout mirrors **[gigs.md](gigs.md) gig roster**: one **table per category**, **
|
|||
|
||||
- **Production chain shape:** **Agreed**—(1) **Overlapping** paths: shared **inputs**, divergent **outputs** across **Process** / **Make**; (2) **Multi-stage** depth: **Gather → Process → Process → Make** (or longer), with **any** **Process** skill able to participate. Recipe UX and balance **open** (roster intro + **Process** subsection in [Skill roster](#skill-roster-draft-ideas-by-category)).
|
||||
- **Agreed Process boundaries:** **Salvage** vs **Quarry**; **Refine** vs **Synth-chem prep** (physical materials vs bio/chem); **Refine** vs **Fab tech** (base material grades vs physical electromechanical fab); **Fab tech** vs **Data scrub** (physical components vs **data/firmware** on salvage). Further merge/split ideas — **open** as the roster evolves.
|
||||
- **Next pass:** pick **prototype trio** (one Gather, one Process/Make, one Tech) for Epic 2/3 slices—written anchors for gather/craft legs: [gathering.md](gathering.md), [crafting.md](crafting.md); lock **`SkillDef` ids** in data with [E2.M1](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md) when Slice 1 freezes.
|
||||
- **Prototype trio (Slice 1 — frozen):** **`salvage`** (Gather), **`refine`** (Process), **`intrusion`** (Tech) — canonical in [`content/skills/prototype_skills.json`](../../content/skills/prototype_skills.json); roster policy and XP channels in [E2.M1](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md). Written anchors for gather/craft legs: [gathering.md](gathering.md), [crafting.md](crafting.md).
|
||||
|
||||
### XP, levels, curves
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,88 @@
|
|||
# NEO-33 — Implementation plan
|
||||
|
||||
## Story reference
|
||||
|
||||
| Field | Value |
|
||||
|--------|--------|
|
||||
| **Key** | NEO-33 |
|
||||
| **Title** | E2.M1: Lock prototype SkillDef trio + CI catalog contract |
|
||||
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-33/e2m1-lock-prototype-skilldef-trio-ci-catalog-contract |
|
||||
|
||||
## Kickoff clarifications
|
||||
|
||||
| Topic | Question | Answer |
|
||||
|--------|------------|--------|
|
||||
| Prototype `SkillDef.id` freeze | Lock **salvage**, **refine**, **intrusion** (gather + process + tech)? | **Yes** — freeze these three ids as-is. |
|
||||
| Designer note placement | Where should the short note on `allowedXpSourceKinds` + grant call sites live? | **Both** — brief pointer in `content/README.md`, fuller text in `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md`. |
|
||||
| `allowedXpSourceKinds` meaning | (User asked in chat.) | Explained in session: per-skill allowlist for XP grant channels (`activity`, `mission_reward`, `trainer`, `book_or_item`); E2.M2 rejects grants outside the set; combat encounter XP stays on gigs, not skills. |
|
||||
|
||||
## Goal, scope, and out-of-scope
|
||||
|
||||
**Goal:** Treat the repo as the source of truth for the **Slice 1 prototype skill catalog**: exactly **three** non-combat `SkillDef` rows covering **gather**, **process** (or make), and **tech**, with stable **`id`** values, validated on every PR against `content/schemas/skill-def.schema.json`, and documented for designers and downstream issues (NEO-34+).
|
||||
|
||||
**In scope (from Linear):**
|
||||
|
||||
- Align `content/skills/prototype_skills.json` (and schema only if contract gaps appear) with [`docs/game-design/skills.md`](../../game-design/skills.md) prototype trio policy (one Gather, one Process/Make, one Tech).
|
||||
- Confirm [`.github/workflows/pr-gate.yml`](../../.github/workflows/pr-gate.yml) runs [`scripts/validate_content.py`](../../scripts/validate_content.py); close small gaps if any (e.g. clearer validation errors, doc pointers).
|
||||
- Short designer note (**README + E2.M1** per kickoff): each row **must** declare non-empty `allowedXpSourceKinds` and what each kind implies for grant call sites.
|
||||
- Call out the **frozen** `id` list in-repo for downstream references (E2.M2, E3, quests).
|
||||
|
||||
**Out of scope (from Linear):**
|
||||
|
||||
- Server runtime catalog load (NEO-34+).
|
||||
- XP grant implementation (E2.M2 / NEO follow-ups).
|
||||
|
||||
## Acceptance criteria checklist
|
||||
|
||||
- [x] PR / CI fails on invalid catalog rows, schema mismatch, or **duplicate** skill `id` across `content/skills/*.json`.
|
||||
- [x] Exactly **three** prototype skills with category coverage **gather** + **process** (or make) + **tech**; **`id`** values **salvage**, **refine**, **intrusion** recorded as **stable / frozen** in decomposition or authoring docs.
|
||||
- [x] CT.M1 and E2.M1 doc pointers remain accurate relative to what ships in this story.
|
||||
- [x] Designer-facing note on `allowedXpSourceKinds` appears in **`content/README.md`** (brief) and **`E2_M1_SkillDefinitionRegistry.md`** (detail).
|
||||
|
||||
## Technical approach
|
||||
|
||||
1. **Catalog:** Keep `content/skills/prototype_skills.json` as the single skills JSON under `content/skills/` for the prototype trio (already matches gather / process / tech). Adjust rows or schema only if validation or design review finds drift from `skills.md` policy.
|
||||
2. **CI:** Rely on existing `validate_content.py` + PR gate; optionally tighten messages or add explicit checks (e.g. total distinct `id` count equals three for Slice 1) if product wants fail-fast on accidental extra files—only if agreed during implementation without scope creep.
|
||||
3. **Docs:** Update **E2.M1** with a **Prototype Slice 1 freeze** subsection: frozen ids, category coverage, and a **grant channels** subsection expanding `allowedXpSourceKinds` for designers (link to schema enum). Update **content/README.md** with a short pointer to that section and a one-line “do not rename ids” reminder.
|
||||
4. **Cross-doc:** Update `skills.md` “Next pass” / prototype language so it points at the **locked** ids (no longer an open pick). Refresh [`documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) E2.M1 row if it still implies NEO-33 work is only backlog.
|
||||
5. **No application server or C# changes** in this story unless an unexpected coupling appears (unlikely).
|
||||
|
||||
## Files to add
|
||||
|
||||
| Path | Purpose |
|
||||
|------|---------|
|
||||
| **None** | All contracts and catalogs already exist; this story is documentation alignment, optional validation tightening, and explicit freeze callouts—not new modules. |
|
||||
|
||||
## Files to modify
|
||||
|
||||
| Path | Rationale |
|
||||
|------|------------|
|
||||
| `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | Add frozen trio + designer-oriented `allowedXpSourceKinds` / grant call-site guidance (canonical). |
|
||||
| `content/README.md` | Brief pointer to E2.M1 detail and freeze reminder for content authors. |
|
||||
| `docs/game-design/skills.md` | Point “prototype trio” at locked ids so design doc matches data. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | Update E2.M1 / NEO-33 row to match shipped state after implementation. |
|
||||
| `scripts/validate_content.py` | Shipped **NEO-33** Slice 1 gate: exact frozen ids + category coverage after schema + duplicate-id checks. |
|
||||
|
||||
## Tests
|
||||
|
||||
| Item | Coverage |
|
||||
|------|----------|
|
||||
| **Automated** | Existing **PR gate** job runs `python scripts/validate_content.py` — primary regression signal for schema + duplicate `id`. No dedicated pytest module exists in-repo today; introducing one is **optional** and not required for NEO-33 acceptance if CI + local script remain the contract. |
|
||||
| **Manual** | Run `pip install -r scripts/requirements-content.txt && python scripts/validate_content.py` locally after doc/catalog edits; skim PR gate workflow in GitHub on the story PR. |
|
||||
|
||||
## Open questions / risks
|
||||
|
||||
- **Catalog growth:** `validate_content.py` now requires **exactly** the three frozen ids; expanding the roster requires a follow-up issue to relax or replace this gate.
|
||||
- **None** otherwise at kickoff.
|
||||
|
||||
## Decisions
|
||||
|
||||
| Decision | Rationale |
|
||||
|----------|------------|
|
||||
| Frozen ids **salvage**, **refine**, **intrusion** | User confirmation during kickoff. |
|
||||
| Designer note in **README + E2.M1** | User picked option **3**. |
|
||||
|
||||
## Implementation notes (shipped)
|
||||
|
||||
- **`scripts/validate_content.py`:** after schema validation, enforces `PROTOTYPE_SLICE1_SKILL_IDS` and category coverage (gather, tech, process or make).
|
||||
- **Docs:** E2.M1 designer note + freeze table; `content/README` pointer; `skills.md` frozen trio bullet; CT.M1 CI paragraph; alignment + dependency register **E2.M1** → **In Progress**.
|
||||
|
|
@ -0,0 +1,51 @@
|
|||
# Code review — NEO-33 (prototype SkillDef trio + CI catalog contract)
|
||||
|
||||
**Date:** 2026-05-02
|
||||
|
||||
**Scope:** Branch `NEO-33-e2m1-lock-prototype-skilldef-trio-ci-catalog-contract` vs `origin/main` (commits `66150ea`, `583201c`: implementation plan, `validate_content.py` Slice 1 gate, E2.M1 / CT.M1 / alignment / register / `content/README` / `skills.md`).
|
||||
|
||||
**Base:** `origin/main` at merge-base `807812148ffa445d3628e6fed1337d53790c3c77`.
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approve with nits**
|
||||
|
||||
## Summary
|
||||
|
||||
The branch implements NEO-33 as documented: a post-schema **prototype Slice 1** gate in `scripts/validate_content.py` enforces the frozen id set (`salvage`, `refine`, `intrusion`) and category coverage (gather, tech, process or make), with decomposition and authoring docs updated so designers see `allowedXpSourceKinds` semantics and the freeze table. `prototype_skills.json` on `main` already matched the trio; no catalog diff is expected. Risk is low (CI + Python script only); one small validator behavior affects error messaging in an edge case, not happy-path correctness.
|
||||
|
||||
## Documentation checked
|
||||
|
||||
| Document | Result |
|
||||
|----------|--------|
|
||||
| `docs/plans/NEO-33-implementation-plan.md` | **Matches** — plan, shipped notes, acceptance checklist, and “Files to modify” align with the diff (catalog unchanged on branch because it was already correct; gate + docs carry the story). |
|
||||
| `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | **Matches** — Status **In Progress**, designer note, freeze table, CI pointer, `allowedXpSourceKinds` table + narrative; schema link remains in **Data contract** above the new subsection. |
|
||||
| `docs/decomposition/modules/CT_M1_ContentValidationPipeline.md` | **Matches** — CI paragraph describes duplicate-id check and NEO-33 trio + category enforcement. |
|
||||
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E2.M1 **In Progress** and E2.M1 note describe NEO-33 → NEO-34+ flow. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E2.M1 row documents NEO-33 landed state and follow-on issues. |
|
||||
| `docs/game-design/skills.md` | **Matches** — Prototype trio bullet points at frozen ids and E2.M1 / catalog anchors. |
|
||||
| `content/README.md` | **Matches** — Brief Slice 1 pointer and anchor to E2.M1 designer section. |
|
||||
| `docs/decomposition/modules/contracts.md` | **N/A** — no new contract kind beyond existing JSON Schema + CI story; consistent with CT.M1 wording. |
|
||||
| `docs/decomposition/modules/client_server_authority.md` | **N/A** — no runtime authority change. |
|
||||
|
||||
**Register / tracking:** Branch already updates E2.M1 to **In Progress** and the alignment row; no further register change required for merge.
|
||||
|
||||
## Blocking issues
|
||||
|
||||
(none)
|
||||
|
||||
## Suggestions
|
||||
|
||||
1. ~~**`validate_content.py` — reserve `seen_ids` only for schema-clean rows** — Today, a string `id` is inserted into `seen_ids` even when `row_schema_errors > 0`, while `id_to_category` is gated on `row_schema_errors == 0`. If an author fixes a broken row and reuses an `id` that appeared only on an invalid earlier row, the validator can report **duplicate skill id** instead of focusing on the remaining schema errors. Consider only updating `seen_ids` when `row_schema_errors == 0` (mirror the `id_to_category` guard) so duplicate detection applies to validated rows only.~~ **Done.**
|
||||
|
||||
2. ~~**E2.M1 acceptance criteria (optional doc polish)** — The bullet that says “when Slice 1 locks three skills” could name **`salvage` / `refine` / `intrusion`** explicitly now that NEO-33 shipped, so the acceptance section matches the freeze subsection without reading both.~~ **Done.**
|
||||
|
||||
## Nits
|
||||
|
||||
- **Nit:** Ensure the PR description mentions that **`content/skills/prototype_skills.json`** was already compliant on `main` so reviewers do not hunt for a missing catalog diff.
|
||||
|
||||
## Verification
|
||||
|
||||
- `pip install -r scripts/requirements-content.txt && python3 scripts/validate_content.py` (expect `content OK` and exit code 0).
|
||||
- Confirm **PR gate** on the PR runs the same script (`.github/workflows/pr-gate.yml` already invokes `python scripts/validate_content.py`).
|
||||
- Optional: add a second skills JSON under `content/skills/` in a throwaway branch to confirm duplicate-id and “exactly three ids” failures read clearly in CI logs.
|
||||
|
|
@ -17,6 +17,31 @@ REPO_ROOT = Path(__file__).resolve().parent.parent
|
|||
SKILL_SCHEMA = REPO_ROOT / "content/schemas/skill-def.schema.json"
|
||||
SKILLS_DIR = REPO_ROOT / "content/skills"
|
||||
|
||||
# Slice 1 prototype lock (NEO-33): exact ids + category coverage after schema passes.
|
||||
PROTOTYPE_SLICE1_SKILL_IDS = frozenset({"salvage", "refine", "intrusion"})
|
||||
|
||||
|
||||
def _prototype_slice1_gate(seen_ids: dict[str, str], id_to_category: dict[str, str]) -> str | None:
|
||||
"""Return a human-readable error if Slice 1 contract fails, else None."""
|
||||
ids = frozenset(seen_ids.keys())
|
||||
if ids != PROTOTYPE_SLICE1_SKILL_IDS:
|
||||
return (
|
||||
"error: prototype Slice 1 expects exactly skill ids "
|
||||
f"{sorted(PROTOTYPE_SLICE1_SKILL_IDS)!r}, got {sorted(ids)!r}"
|
||||
)
|
||||
cats = set(id_to_category.values())
|
||||
if "gather" not in cats or "tech" not in cats:
|
||||
return (
|
||||
"error: prototype Slice 1 requires at least one gather and one tech skill; "
|
||||
f"categories seen: {sorted(cats)!r}"
|
||||
)
|
||||
if "process" not in cats and "make" not in cats:
|
||||
return (
|
||||
"error: prototype Slice 1 requires at least one process or make skill; "
|
||||
f"categories seen: {sorted(cats)!r}"
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def main() -> int:
|
||||
if not SKILL_SCHEMA.is_file():
|
||||
|
|
@ -36,6 +61,7 @@ def main() -> int:
|
|||
return 1
|
||||
|
||||
seen_ids: dict[str, str] = {}
|
||||
id_to_category: dict[str, str] = {}
|
||||
errors = 0
|
||||
|
||||
for path in json_files:
|
||||
|
|
@ -50,15 +76,19 @@ def main() -> int:
|
|||
print(f"error: {path.relative_to(REPO_ROOT)}: skills[{i}] must be an object", file=sys.stderr)
|
||||
errors += 1
|
||||
continue
|
||||
row_schema_errors = 0
|
||||
for err in sorted(validator.iter_errors(row), key=lambda e: e.path):
|
||||
loc = ".".join(str(p) for p in err.path) or "(root)"
|
||||
print(
|
||||
f"error: {path.relative_to(REPO_ROOT)} skills[{i}] {loc}: {err.message}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
row_schema_errors += 1
|
||||
errors += 1
|
||||
sid = row.get("id")
|
||||
if isinstance(sid, str):
|
||||
# Reserve seen_ids (and duplicate detection) only for schema-clean rows so
|
||||
# fixing a broken row does not surface a spurious duplicate vs an earlier invalid row.
|
||||
if isinstance(sid, str) and row_schema_errors == 0:
|
||||
prev = seen_ids.get(sid)
|
||||
if prev:
|
||||
print(
|
||||
|
|
@ -68,11 +98,19 @@ def main() -> int:
|
|||
errors += 1
|
||||
else:
|
||||
seen_ids[sid] = str(path.relative_to(REPO_ROOT))
|
||||
cat = row.get("category")
|
||||
if isinstance(cat, str):
|
||||
id_to_category[sid] = cat
|
||||
|
||||
if errors:
|
||||
print(f"content validation failed with {errors} error(s)", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
slice1_err = _prototype_slice1_gate(seen_ids, id_to_category)
|
||||
if slice1_err:
|
||||
print(slice1_err, file=sys.stderr)
|
||||
return 1
|
||||
|
||||
print(f"content OK: {len(json_files)} skill catalog file(s), {len(seen_ids)} unique skill id(s)")
|
||||
return 0
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue