From 6cb7dfbb46da66cae9449d057dbd9def9aaec578 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 15 Jun 2026 19:52:42 -0400 Subject: [PATCH] =?UTF-8?q?NEO-133:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20roster=20docs,=20CI=20cleanup,=20band=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-link E7.M1 doc to E7M3 five-quest roster; remove dead _prototype_e7m1_quest_gate; add faction standing-band CI gate; clarify server roster error strings; grid-contract loader test asserts. --- .../modules/E7_M1_QuestStateMachine.md | 2 +- docs/reviews/2026-06-15-NEO-133.md | 14 +++---- scripts/validate_content.py | 39 ++++++++++++------- .../QuestDefinitionCatalogLoaderTests.cs | 10 ++++- .../Quests/PrototypeE7M1QuestCatalogRules.cs | 13 ++++--- 5 files changed, 50 insertions(+), 28 deletions(-) diff --git a/docs/decomposition/modules/E7_M1_QuestStateMachine.md b/docs/decomposition/modules/E7_M1_QuestStateMachine.md index c931843..9e604f1 100644 --- a/docs/decomposition/modules/E7_M1_QuestStateMachine.md +++ b/docs/decomposition/modules/E7_M1_QuestStateMachine.md @@ -76,7 +76,7 @@ The **first shipped quest spine** is **frozen** for prototype tuning until a del | **`prototype_quest_combat_intro`** | Intro: Clear the Pocket | *(none)* | 1× **`encounter_complete`**: **`prototype_combat_pocket`** | | **`prototype_quest_operator_chain`** | Operator Chain | **`prototype_quest_gather_intro`**, **`prototype_quest_refine_intro`**, **`prototype_quest_combat_intro`** | 4 steps: gather **`scrap_metal_bulk` ×5** → craft **`refine_scrap_standard`** → craft **`make_field_stim_mk0`** → **`inventory_has_item`** **`contract_handoff_token` ×1** | -**CI enforcement (NEO-112):** `scripts/validate_content.py` requires **exactly** these four quest ids; objective cross-refs to frozen item/recipe/encounter catalogs; acyclic **`prerequisiteQuestIds`**; chain quest terminal step uses **`contract_handoff_token`** from [E5.M3 encounter loot](E5_M3_EncounterAndRewardTables.md#prototype-slice-3-freeze-e5m3-01). +**CI enforcement (NEO-112):** `scripts/validate_content.py` still enforces the **four Slice 1 quest ids** in the freeze table above (prerequisite graph, objective cross-refs, chain terminal step). **Roster allowlist** expanded to **five** quests under [E7.M3](E7_M3_FactionReputationLedger.md) (`PROTOTYPE_E7M3_QUEST_IDS`, NEO-133) — adds **`prototype_quest_grid_contract`**. Shared gates: objective cross-refs to frozen item/recipe/encounter catalogs; acyclic **`prerequisiteQuestIds`**; operator chain terminal step uses **`contract_handoff_token`** from [E5.M3 encounter loot](E5_M3_EncounterAndRewardTables.md#prototype-slice-3-freeze-e5m3-01). **Server load (NEO-113):** Host fail-fast load of `content/quests/*_quests.json` with the same gates — [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). diff --git a/docs/reviews/2026-06-15-NEO-133.md b/docs/reviews/2026-06-15-NEO-133.md index 1580719..1f53d9a 100644 --- a/docs/reviews/2026-06-15-NEO-133.md +++ b/docs/reviews/2026-06-15-NEO-133.md @@ -19,7 +19,7 @@ This branch lands Epic 7 Slice 3 **content contracts** ahead of server faction l | `docs/plans/NEO-133-implementation-plan.md` | **Matches** — shipped items reconcile with plan (schemas, catalog, CI, server parity, docs). | | `docs/plans/E7M3-pre-production-backlog.md` | **Matches** — E7M3-01 AC checked; client capstone NEO-142/143 documented. | | `docs/decomposition/modules/E7_M3_FactionReputationLedger.md` | **Matches** — Slice 3 freeze table and CI line updated. | -| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Partially matches** — Slice 1 freeze table still documents **four** quests; CI enforcement paragraph still says “exactly these four quest ids” without noting E7M3 roster expansion (see Suggestions). | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — Slice 1 freeze table preserved; CI paragraph cross-links E7.M3 five-quest roster. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7.M3 status In Progress, E7.M2 dependency added. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M3 row added. | | `docs/decomposition/epics/epic_07_quest_faction.md` | **Matches** — Slice 3 backlog link and Godot capstone AC. | @@ -34,18 +34,18 @@ None. ## Suggestions -1. **E7.M1 module doc vs five-quest CI** — `E7_M1_QuestStateMachine.md` still states CI requires exactly the four Slice 1 quest ids. Add a one-line cross-link to E7.M3 (`PROTOTYPE_E7M3_QUEST_IDS`) so readers do not assume the four-quest gate is still the active roster check. +1. ~~**E7.M1 module doc vs five-quest CI** — `E7_M1_QuestStateMachine.md` still states CI requires exactly the four Slice 1 quest ids. Add a one-line cross-link to E7.M3 (`PROTOTYPE_E7M3_QUEST_IDS`) so readers do not assume the four-quest gate is still the active roster check.~~ **Done.** CI paragraph cross-links E7.M3 roster expansion. -2. **Server constant naming** — `PrototypeE7M1QuestCatalogRules.ExpectedQuestIds` and `TryGetE7M1GateError` now enforce **five** ids while labels/messages still say “E7M1”. When NEO-134 lands, consider `PrototypeE7M3QuestCatalogRules` (or rename + update error strings) to avoid onboarding confusion. +2. ~~**Server constant naming** — `PrototypeE7M1QuestCatalogRules.ExpectedQuestIds` and `TryGetE7M1GateError` now enforce **five** ids while labels/messages still say “E7M1”. When NEO-134 lands, consider `PrototypeE7M3QuestCatalogRules` (or rename + update error strings) to avoid onboarding confusion.~~ **Done (partial).** Error string now says “prototype quest roster”; class/XML docs note NEO-134 rename. Full type rename deferred to NEO-134. -3. **Remove dead CI helper** — `_prototype_e7m1_quest_gate` in `scripts/validate_content.py` is no longer called (superseded by `_prototype_e7m3_quest_roster_gate`). Delete or mark deprecated to prevent accidental reintroduction of the four-quest gate. +3. ~~**Remove dead CI helper** — `_prototype_e7m1_quest_gate` in `scripts/validate_content.py` is no longer called (superseded by `_prototype_e7m3_quest_roster_gate`). Delete or mark deprecated to prevent accidental reintroduction of the four-quest gate.~~ **Done.** Dead helper removed. -4. **Faction standing band schema** — `faction-def.schema.json` does not constrain `minStanding <= neutralStanding <= maxStanding`. Optional JSON Schema `minimum`/`maximum` cross-field checks (or a CI gate mirroring faction freeze) would catch designer mistakes before NEO-134 server load. +4. ~~**Faction standing band schema** — `faction-def.schema.json` does not constrain `minStanding <= neutralStanding <= maxStanding`. Optional JSON Schema `minimum`/`maximum` cross-field checks (or a CI gate mirroring faction freeze) would catch designer mistakes before NEO-134 server load.~~ **Done.** `_prototype_e7m3_faction_standing_band_gate` enforces ordering on all faction rows. ## Nits -- Nit: `PROTOTYPE_E7M1_QUEST_IDS` (four ids) remains beside `PROTOTYPE_E7M3_QUEST_IDS` (five ids); fine for historical Slice 1 reference, but a short comment on E7M1 constant (“Slice 1 roster subset; CI roster gate is E7M3”) would clarify intent. -- Nit: `QuestDefinitionCatalogLoaderTests.Load_ShouldSucceed_WhenCatalogMatchesPrototypeContract` asserts gather/chain rows but does not assert grid-contract gate/bundle fields; acceptable per plan (“no new Fact unless regression needs it”), but a single assert on grid-contract prereq/gate would cheaply lock the fifth quest shape server-side. +- ~~Nit: `PROTOTYPE_E7M1_QUEST_IDS` (four ids) remains beside `PROTOTYPE_E7M3_QUEST_IDS` (five ids); fine for historical Slice 1 reference, but a short comment on E7M1 constant (“Slice 1 roster subset; CI roster gate is E7M3”) would clarify intent.~~ **Done.** +- ~~Nit: `QuestDefinitionCatalogLoaderTests.Load_ShouldSucceed_WhenCatalogMatchesPrototypeContract` asserts gather/chain rows but does not assert grid-contract gate/bundle fields; acceptable per plan (“no new Fact unless regression needs it”), but a single assert on grid-contract prereq/gate would cheaply lock the fifth quest shape server-side.~~ **Done.** Asserts grid-contract prereq and rep-only bundle (item/skill empty at server parse layer). ## Verification diff --git a/scripts/validate_content.py b/scripts/validate_content.py index 7d67034..2e48a60 100644 --- a/scripts/validate_content.py +++ b/scripts/validate_content.py @@ -166,8 +166,9 @@ PROTOTYPE_E5M3_REWARD_GRANTS: dict[str, int] = { "contract_handoff_token": 1, } -# Epic 7 Slice 1 prototype lock (NEO-112): exact quest ids after schema passes. -# Keep in sync with E7.M1 freeze table and future NEO-113 server loader. +# Epic 7 Slice 1 prototype lock (NEO-112): Slice 1 onboarding quest ids (historical subset). +# Active CI roster gate is PROTOTYPE_E7M3_QUEST_IDS (five quests, NEO-133). +# Keep in sync with E7.M1 freeze table; server ExpectedQuestIds expanded for E7M3. PROTOTYPE_E7M1_QUEST_IDS = frozenset( { "prototype_quest_gather_intro", @@ -1153,17 +1154,6 @@ def _prototype_e7m3_quest_roster_gate(seen_ids: dict[str, str]) -> str | None: return None -def _prototype_e7m1_quest_gate(seen_ids: dict[str, str]) -> str | None: - """Return a human-readable error if E7M1 quest contract fails, else None.""" - ids = frozenset(seen_ids.keys()) - if ids != PROTOTYPE_E7M1_QUEST_IDS: - return ( - "error: prototype E7M1 expects exactly quest ids " - f"{sorted(PROTOTYPE_E7M1_QUEST_IDS)!r}, got {sorted(ids)!r}" - ) - return None - - def _prototype_e7m1_quest_prerequisite_gate(id_to_row: dict[str, dict]) -> str | None: """Return a human-readable error if prerequisite ids are unknown or cyclic, else None.""" known = frozenset(id_to_row.keys()) @@ -1387,6 +1377,24 @@ def _prototype_e7m3_faction_freeze_gate(id_to_row: dict[str, dict]) -> str | Non return None +def _prototype_e7m3_faction_standing_band_gate(id_to_row: dict[str, dict]) -> str | None: + """Return a human-readable error if faction standing band ordering fails, else None.""" + for fid, row in id_to_row.items(): + if not isinstance(row, dict): + continue + min_standing = row.get("minStanding") + max_standing = row.get("maxStanding") + neutral_standing = row.get("neutralStanding") + if not all(isinstance(value, int) for value in (min_standing, max_standing, neutral_standing)): + continue + if min_standing > neutral_standing or neutral_standing > max_standing: + return ( + f"error: faction {fid!r} requires minStanding <= neutralStanding <= maxStanding " + f"(got min={min_standing!r}, neutral={neutral_standing!r}, max={max_standing!r})" + ) + return None + + def _prototype_e7m3_faction_cross_ref_gate( id_to_row: dict[str, dict], faction_seen_ids: dict[str, str], @@ -2284,6 +2292,11 @@ def main() -> int: print(e7m3_faction_freeze_err, file=sys.stderr) return 1 + e7m3_faction_band_err = _prototype_e7m3_faction_standing_band_gate(faction_id_to_row) + if e7m3_faction_band_err: + print(e7m3_faction_band_err, file=sys.stderr) + return 1 + quest_errors, quest_seen_ids, quest_id_to_row = _validate_quest_catalogs( quest_files=quest_files, quest_validator=quest_validator, diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs index 50edc16..8dc20c6 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionCatalogLoaderTests.cs @@ -302,6 +302,14 @@ public class QuestDefinitionCatalogLoaderTests Assert.Single(gather.CompletionRewardBundle!.SkillXpGrants); Assert.Equal("salvage", gather.CompletionRewardBundle.SkillXpGrants[0].SkillId); Assert.Equal(25, gather.CompletionRewardBundle.SkillXpGrants[0].Amount); + Assert.True(catalog.TryGetQuest(PrototypeE7M1QuestCatalogRules.GridContractQuestId, out var gridContract)); + Assert.NotNull(gridContract); + Assert.Equal( + [PrototypeE7M1QuestCatalogRules.ChainQuestId], + gridContract!.PrerequisiteQuestIds); + Assert.NotNull(gridContract.CompletionRewardBundle); + Assert.Empty(gridContract.CompletionRewardBundle!.ItemGrants); + Assert.Empty(gridContract.CompletionRewardBundle.SkillXpGrants); } [Fact] @@ -471,7 +479,7 @@ public class QuestDefinitionCatalogLoaderTests var ex = Record.Exception(() => LoadCatalog(questsDir, defSchemaPath, stepSchemaPath, objectiveSchemaPath)); // Assert var ioe = Assert.IsType(ex); - Assert.Contains("prototype E7M1 expects exactly quest ids", ioe.Message, StringComparison.Ordinal); + Assert.Contains("prototype quest roster expects exactly quest ids", ioe.Message, StringComparison.Ordinal); } [Fact] diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs index e0512b7..5b93314 100644 --- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs +++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs @@ -3,14 +3,15 @@ using System.Collections.Frozen; namespace NeonSprawl.Server.Game.Quests; /// -/// Prototype E7M1 quest roster + cross-ref gates (NEO-113), mirrored from -/// scripts/validate_content.py PROTOTYPE_E7M1_QUEST_IDS, +/// Prototype quest roster + cross-ref gates (NEO-113, NEO-133), mirrored from +/// scripts/validate_content.py PROTOTYPE_E7M3_QUEST_IDS (five ids), /// PROTOTYPE_E7M1_CHAIN_QUEST_ID, PROTOTYPE_E7M1_CHAIN_TERMINAL_ITEM_ID, -/// and _prototype_e7m1_* gate functions. +/// and _prototype_e7m1_* / _prototype_e7m3_* gate functions. +/// Class name retained for NEO-113 call sites; rename to PrototypeE7M3QuestCatalogRules planned in NEO-134. /// public static class PrototypeE7M1QuestCatalogRules { - /// Keep in sync with scripts/validate_content.py PROTOTYPE_E7M1_QUEST_IDS. + /// Keep in sync with scripts/validate_content.py PROTOTYPE_E7M3_QUEST_IDS (five quests). public static readonly FrozenSet ExpectedQuestIds = FrozenSet.ToFrozenSet( [ "prototype_quest_gather_intro", @@ -45,14 +46,14 @@ public static class PrototypeE7M1QuestCatalogRules /// Keep in sync with scripts/validate_content.py PROTOTYPE_E7M1_CHAIN_TERMINAL_ITEM_ID. public const string ChainTerminalItemId = "contract_handoff_token"; - /// Returns a human-readable error if the E7M1 quest contract fails, otherwise . + /// Returns a human-readable error if the prototype quest roster fails, otherwise . public static string? TryGetE7M1GateError(IReadOnlyDictionary questIdToSourceFile) { var ids = questIdToSourceFile.Keys.ToFrozenSet(StringComparer.Ordinal); if (!ids.SetEquals(ExpectedQuestIds)) { return - "error: prototype E7M1 expects exactly quest ids " + + "error: prototype quest roster expects exactly quest ids " + $"[{string.Join(", ", ExpectedQuestIds.Order(StringComparer.Ordinal).Select(s => "'" + s + "'"))}], " + $"got [{string.Join(", ", ids.Order(StringComparer.Ordinal).Select(s => "'" + s + "'"))}]"; }