fix(contract): repair 3-arg LogPlayer callers + digest article bug
Three callers (main_window_shell.gd, narrate_harness.gd, npc_harness.gd)
still built LogPlayer with the OLD 3-arg shape (name, class_id,
luck_descriptor) after LogPlayer.new() was migrated to the 4-arg
(name, race_id, calling_id, luck_descriptor) shape. This was invisible
to both grep and the test suite: every ctor param carries a default, so
GDScript compiles a 3-arg call against a 4-arg signature without error —
it just silently mis-binds positionally. "sellsword" landed in the
race_id slot (rejected — not a race), the luck descriptor landed in the
calling_id slot (rejected — not a calling), and luck_descriptor itself
fell back to its default "". Every field in the emitted dict came out
empty, which 422s against the canon-log schema's minLength/enum
constraints. Grepping for class_id can't see this, because there is no
class_id token left anywhere — the bug is purely positional.
Also fixes prompts.py's _article(""), which returned "an" because
Python's "" in "aeiou" is True, producing a doubled-space/wrong-article
digest line when race_id is empty. _describe_player now builds its
descriptor from whichever of race/calling are present and omits the
clause entirely when both are absent.
Adds a schema-parity guard for origin.schema.json's inlined
allowed_callings enum, which duplicated the calling roster with no
runtime check tying it to Callings.IDS. Renames leftover "class"
vocabulary in test names/comments to "calling".
Regression coverage:
- client/tests/unit/test_entities.gd: ctor populates both race_id and
calling_id; a calling passed into the race_id slot (the exact shape
of the three broken call sites) yields an all-empty row.
- api/tests/test_prompts.py: empty-race and empty-race-and-calling
digest rendering, asserting no doubled space and no dangling article.
- client/tests/unit/test_schema_parity.gd: origin.schema.json's
allowed_callings enum matches Callings.IDS.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYa9u7Kdxv5gX4AnwWexy8
This commit is contained in:
@@ -18,6 +18,8 @@ def _humanize(id_: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def _article(word: str) -> str:
|
def _article(word: str) -> str:
|
||||||
|
if not word:
|
||||||
|
return "a"
|
||||||
return "an" if word[:1].lower() in "aeiou" else "a"
|
return "an" if word[:1].lower() in "aeiou" else "a"
|
||||||
|
|
||||||
|
|
||||||
@@ -26,7 +28,11 @@ def _describe_player(player: dict) -> str:
|
|||||||
race = _humanize(player.get("race_id", ""))
|
race = _humanize(player.get("race_id", ""))
|
||||||
calling = _humanize(player.get("calling_id", ""))
|
calling = _humanize(player.get("calling_id", ""))
|
||||||
name = player.get("name", "")
|
name = player.get("name", "")
|
||||||
return f"{name}, {_article(race)} {race} {calling}".strip()
|
parts = [p for p in (race, calling) if p]
|
||||||
|
if not parts:
|
||||||
|
return name
|
||||||
|
descriptor = " ".join(parts)
|
||||||
|
return f"{name}, {_article(parts[0])} {descriptor}".strip()
|
||||||
|
|
||||||
|
|
||||||
@lru_cache(maxsize=None)
|
@lru_cache(maxsize=None)
|
||||||
|
|||||||
@@ -30,7 +30,7 @@ def test_null_start_quest_is_allowed():
|
|||||||
assert validate_origin(doc) == []
|
assert validate_origin(doc) == []
|
||||||
|
|
||||||
|
|
||||||
def test_unknown_class_is_rejected():
|
def test_unknown_calling_is_rejected():
|
||||||
doc = copy.deepcopy(VALID_ORIGIN)
|
doc = copy.deepcopy(VALID_ORIGIN)
|
||||||
doc["build_constraints"]["allowed_callings"] = ["bard"]
|
doc["build_constraints"]["allowed_callings"] = ["bard"]
|
||||||
assert validate_origin(doc) != []
|
assert validate_origin(doc) != []
|
||||||
|
|||||||
@@ -64,6 +64,21 @@ def test_digest_does_not_leak_a_snake_case_id():
|
|||||||
assert "hedge-mage" in out
|
assert "hedge-mage" in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_digest_omits_race_clause_when_race_is_empty():
|
||||||
|
# Regression: _article("") used to return "an" (Python's "" in "aeiou" is
|
||||||
|
# True), rendering "Player: Aldric, an " — wrong article, doubled space.
|
||||||
|
out = render_digest(_with_player(name="Aldric", race_id="", calling_id="sellsword"))
|
||||||
|
assert "Player: Aldric, a sellsword" in out
|
||||||
|
assert " " not in out
|
||||||
|
assert ", an " not in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_digest_omits_both_clauses_when_race_and_calling_are_empty():
|
||||||
|
out = render_digest(_with_player(name="Aldric", race_id="", calling_id=""))
|
||||||
|
assert "\nPlayer: Aldric\n" in out
|
||||||
|
assert " " not in out
|
||||||
|
|
||||||
|
|
||||||
def test_render_npc_digest_includes_all_sections():
|
def test_render_npc_digest_includes_all_sections():
|
||||||
from app import prompts
|
from app import prompts
|
||||||
log = {
|
log = {
|
||||||
|
|||||||
@@ -59,7 +59,7 @@ func _on_narrate_pressed() -> void:
|
|||||||
|
|
||||||
func _build_scene_log() -> CanonLog:
|
func _build_scene_log() -> CanonLog:
|
||||||
var log := CanonLog.new()
|
var log := CanonLog.new()
|
||||||
log.player = LogPlayer.new("Aldric", "sellsword", "Fortune spits on you")
|
log.player = LogPlayer.new("Aldric", "human", "sellsword", "Fortune spits on you")
|
||||||
log.set_location("greywater_docks", "the Greywater docks")
|
log.set_location("greywater_docks", "the Greywater docks")
|
||||||
log.party.append(PartyMember.new("brannoc_thane", "Brannoc Thane", 40))
|
log.party.append(PartyMember.new("brannoc_thane", "Brannoc Thane", 40))
|
||||||
log.party.append(PartyMember.new("cadwyn_vell", "Cadwyn Vell", 15))
|
log.party.append(PartyMember.new("cadwyn_vell", "Cadwyn Vell", 15))
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ func _ready() -> void:
|
|||||||
# Build the player through LogPlayer so luck_descriptor is non-empty — the
|
# Build the player through LogPlayer so luck_descriptor is non-empty — the
|
||||||
# canon-log schema requires it (minLength 1), and it's the §7 fortune line
|
# canon-log schema requires it (minLength 1), and it's the §7 fortune line
|
||||||
# the digest surfaces to the NPC. An empty string here 422s the request.
|
# the digest surfaces to the NPC. An empty string here 422s the request.
|
||||||
_canon_log.player = LogPlayer.new("Aldric", "sellsword", "the dice are kind today")
|
_canon_log.player = LogPlayer.new("Aldric", "human", "sellsword", "the dice are kind today")
|
||||||
# Deliberately no find_the_ledger quest here — leave it un-started so Fenn
|
# Deliberately no find_the_ledger quest here — leave it un-started so Fenn
|
||||||
# can offer_quest it during the conversation.
|
# can offer_quest it during the conversation.
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ extends RefCounted
|
|||||||
## New-game construction (spec "New-game construction"). Reads three immutable
|
## New-game construction (spec "New-game construction"). Reads three immutable
|
||||||
## inputs (origin, world, creation) and writes two products (CanonLog, GameState).
|
## inputs (origin, world, creation) and writes two products (CanonLog, GameState).
|
||||||
## Nothing flows back up — charter §2 on the client. Validation is front-loaded:
|
## Nothing flows back up — charter §2 on the client. Validation is front-loaded:
|
||||||
## a broken origin or illegal class fails here, loudly, not three scenes later.
|
## a broken origin or illegal calling fails here, loudly, not three scenes later.
|
||||||
|
|
||||||
static func construct(origin: Dictionary, world: ContentDB, creation: Dictionary, rng: RandomNumberGenerator) -> Dictionary:
|
static func construct(origin: Dictionary, world: ContentDB, creation: Dictionary, rng: RandomNumberGenerator) -> Dictionary:
|
||||||
var errors: Array = []
|
var errors: Array = []
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ func _build_seed_log() -> CanonLog:
|
|||||||
# save/load (M9) construct the real one — mirrors narrate_harness. No Luck
|
# save/load (M9) construct the real one — mirrors narrate_harness. No Luck
|
||||||
# number is rendered anywhere (§7); the descriptor is prose-only.
|
# number is rendered anywhere (§7); the descriptor is prose-only.
|
||||||
var log := CanonLog.new()
|
var log := CanonLog.new()
|
||||||
log.player = LogPlayer.new("Vexcca", "sellsword", "Fortune spits on you")
|
log.player = LogPlayer.new("Vexcca", "human", "sellsword", "Fortune spits on you")
|
||||||
log.set_location("lower_ward", "the Lower Ward")
|
log.set_location("lower_ward", "the Lower Ward")
|
||||||
log.party.append(PartyMember.new("brannoc_thane", "Brannoc Thane", 40))
|
log.party.append(PartyMember.new("brannoc_thane", "Brannoc Thane", 40))
|
||||||
log.party.append(PartyMember.new("cadwyn_vell", "Cadwyn Vell", 15))
|
log.party.append(PartyMember.new("cadwyn_vell", "Cadwyn Vell", 15))
|
||||||
|
|||||||
@@ -32,6 +32,33 @@ func test_log_player_rejects_a_dead_calling():
|
|||||||
assert_true(p.set_race_id("dwarf"))
|
assert_true(p.set_race_id("dwarf"))
|
||||||
|
|
||||||
|
|
||||||
|
func test_log_player_ctor_populates_both_race_and_calling():
|
||||||
|
# Regression: three call sites once passed the OLD 3-arg (name, class_id,
|
||||||
|
# luck_descriptor) shape against the NEW 4-arg (name, race_id, calling_id,
|
||||||
|
# luck_descriptor) constructor. Every param has a default, so GDScript
|
||||||
|
# compiled it silently — the string meant as a calling landed in the
|
||||||
|
# race_id slot, got rejected, and both fields came out "". This asserts
|
||||||
|
# a properly-shaped call yields BOTH fields non-empty.
|
||||||
|
var p := LogPlayer.new("Aldric", "human", "sellsword", "Fortune spits on you")
|
||||||
|
var d := p.to_dict()
|
||||||
|
assert_ne(d["race_id"], "", "race_id must not be empty")
|
||||||
|
assert_ne(d["calling_id"], "", "calling_id must not be empty")
|
||||||
|
|
||||||
|
|
||||||
|
func test_log_player_ctor_rejects_calling_passed_as_race():
|
||||||
|
# The exact shape of the bug: calling three broken call sites used to pass —
|
||||||
|
# LogPlayer.new(name, class_id, luck_descriptor) against the NEW ctor
|
||||||
|
# (name, race_id, calling_id, luck_descriptor). "sellsword" lands in the
|
||||||
|
# race_id slot (rejected — not a race), the descriptor lands in the
|
||||||
|
# calling_id slot (rejected — not a calling), and luck_descriptor is left
|
||||||
|
# at its default. Every field ends up "", which 422s against the schema.
|
||||||
|
var p := LogPlayer.new("Aldric", "sellsword", "Fortune spits on you")
|
||||||
|
var d := p.to_dict()
|
||||||
|
assert_eq(d["race_id"], "", "a calling id is not a legal race_id")
|
||||||
|
assert_eq(d["calling_id"], "", "a luck descriptor is not a legal calling_id")
|
||||||
|
assert_eq(d["luck_descriptor"], "", "old 3-arg call shape must not produce a usable player row")
|
||||||
|
|
||||||
|
|
||||||
func test_party_member_clamps_disposition():
|
func test_party_member_clamps_disposition():
|
||||||
var m = PartyMember.new("brannoc_thane", "Brannoc Thane", 200)
|
var m = PartyMember.new("brannoc_thane", "Brannoc Thane", 200)
|
||||||
assert_eq(m.disposition, 100)
|
assert_eq(m.disposition, 100)
|
||||||
|
|||||||
@@ -73,7 +73,7 @@ func test_null_start_quest_yields_no_quest():
|
|||||||
assert_eq(res["log"].active_quests.size(), 0)
|
assert_eq(res["log"].active_quests.size(), 0)
|
||||||
|
|
||||||
|
|
||||||
func test_class_not_allowed_is_rejected():
|
func test_calling_not_allowed_is_rejected():
|
||||||
var res := _build({"name": "X", "race_id": "human", "calling_id": "berserker"})
|
var res := _build({"name": "X", "race_id": "human", "calling_id": "berserker"})
|
||||||
assert_false(res["ok"])
|
assert_false(res["ok"])
|
||||||
assert_true(res["errors"].size() > 0)
|
assert_true(res["errors"].size() > 0)
|
||||||
|
|||||||
@@ -35,3 +35,19 @@ func test_schema_requires_race_and_calling():
|
|||||||
assert_true("race_id" in required)
|
assert_true("race_id" in required)
|
||||||
assert_true("calling_id" in required)
|
assert_true("calling_id" in required)
|
||||||
assert_false("class_id" in required, "class_id is the rulebook's word — it is gone")
|
assert_false("class_id" in required, "class_id is the rulebook's word — it is gone")
|
||||||
|
|
||||||
|
|
||||||
|
func test_origin_schema_allowed_callings_matches_the_code():
|
||||||
|
# origin.schema.json's build_constraints.allowed_callings duplicates the
|
||||||
|
# same seven calling ids inline (a JSON Schema "enum" cannot $ref another
|
||||||
|
# file's enum), so nothing at runtime reconciles it with Callings.IDS
|
||||||
|
# either. This guards it the same way _player_schema() guards canon-log.
|
||||||
|
var path := ProjectSettings.globalize_path("res://") \
|
||||||
|
.path_join("../docs/schemas/origin.schema.json").simplify_path()
|
||||||
|
var doc: Variant = ContentDB.load_json(path)
|
||||||
|
assert_eq(typeof(doc), TYPE_DICTIONARY, "origin schema did not parse")
|
||||||
|
var enum_ids: Array = doc["properties"]["build_constraints"]["properties"]["allowed_callings"]["items"]["enum"]
|
||||||
|
enum_ids.sort()
|
||||||
|
var code_ids: Array = Callings.IDS.duplicate()
|
||||||
|
code_ids.sort()
|
||||||
|
assert_eq(enum_ids, code_ids)
|
||||||
|
|||||||
Reference in New Issue
Block a user