Compare commits

...

2 Commits

Author SHA1 Message Date
e8a61bf76f Merge fix/duplicate-move-tags into dev
A move applies at most once per reply. Repeating a tag let the model pick an
amount in unary (§2 breach); the currency-only guard is generalized and deleted.
adjust_disposition's MAX_DELTA now caps the reply's net swing, not each tag.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYa9u7Kdxv5gX4AnwWexy8
2026-07-12 19:11:47 -05:00
acc5a7b2a2 fix(npc): a move applies at most once per reply
The tag extractor returns every occurrence of a move tag and the validator is
a pure membership test, so MoveApplier applied duplicates. A model repeating a
tag N times therefore picked an amount in unary — the AI choosing a number,
which is the §2 breach the bounded vocabulary exists to prevent.

The currency fix (8e1238c) closed this for denominations only. Generalizing it
to every move that names a thing covers real items too — three give_item(amulet)
tags are one amulet, not three — and lets the currency special case be deleted
rather than kept alongside a second mechanism. ID_MOVES is reused from
MoveValidator instead of restated.

Also caps adjust_disposition on the reply's NET swing. MAX_DELTA was clamped
per tag, so three +15s moved standing by 45 — precisely the wholesale swing its
own comment says it forbids. Applied in emission order, so a later
become_hostile still floors what came before it.

offer_quest and reveal needed no change: add_quest self-guards on has_quest and
mark_revealed is idempotent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QYa9u7Kdxv5gX4AnwWexy8
2026-07-12 19:11:40 -05:00
3 changed files with 86 additions and 21 deletions

View File

@@ -11,20 +11,24 @@ const HOSTILE_FLOOR := -100
static func apply(moves: Array, game_state, canon_log, content_db, npc_id: String) -> void:
# A currency move (give_item/accept_item on a denomination) applies AT MOST
# ONCE per reply. Without this, a model repeating one tag N times would pick
# an arbitrary amount in unary — the AI choosing a number is a §2 breach.
# Real-item duplicates are unaffected; that stays bounded by gifts_given.
var currency_applied_this_reply := {} # "name(denom)" -> true
# ONE RULE: a move applies AT MOST ONCE PER REPLY. The tag extractor returns
# every occurrence and the validator is a membership test, so without this a
# model repeating a tag N times picks an amount in unary — the AI choosing a
# number, which is the §2 breach the bounded vocabulary exists to prevent.
# It is worth the same for real items: three give_item(amulet) tags are one
# amulet, not three.
var applied := {} # "name(id)" -> true
var dispo_total := 0 # the reply's NET disposition swing, capped at MAX_DELTA
for move in moves:
var name: String = move.get("name", "")
var args: Array = move.get("args", [])
if name in ["give_item", "accept_item"] and not args.is_empty() \
and Currency.is_currency(str(args[0])):
var currency_key := "%s(%s)" % [name, str(args[0])]
if currency_applied_this_reply.has(currency_key):
# MoveValidator.ID_MOVES is the one list of moves that name a thing — those
# are the ones a (name, id) key can dedupe. Reused, not restated.
if name in MoveValidator.ID_MOVES and not args.is_empty():
var key := "%s(%s)" % [name, str(args[0])]
if applied.has(key):
continue
currency_applied_this_reply[currency_key] = true
applied[key] = true
match name:
"offer_quest":
var q: String = str(args[0])
@@ -40,9 +44,17 @@ static func apply(moves: Array, game_state, canon_log, content_db, npc_id: Strin
"accept_item":
game_state.take(str(args[0]), 1)
"adjust_disposition":
# MAX_DELTA caps the reply's NET swing, not each tag — clamping per
# tag let three +15s move standing by 45, which is exactly the
# "wholesale swing" the constant is here to forbid. Applied inside
# the loop (not batched at the end) so emission order still holds
# and a later become_hostile floors what came before it.
var delta: int = clampi(int(str(args[0])), -MAX_DELTA, MAX_DELTA)
var capped: int = clampi(dispo_total + delta, -MAX_DELTA, MAX_DELTA)
var allowed: int = capped - dispo_total
dispo_total = capped
var cur: int = int(game_state.npc_dispositions.get(npc_id, 0))
game_state.set_npc_disposition(npc_id, cur + delta)
game_state.set_npc_disposition(npc_id, cur + allowed)
"become_hostile":
game_state.set_npc_disposition(npc_id, HOSTILE_FLOOR)
_:

View File

@@ -144,18 +144,67 @@ func test_dedup_key_is_move_name_not_just_denomination():
assert_eq(gs.purse_copper, 0)
func test_repeated_non_currency_give_item_still_applies_each_time():
# Pins the scope line: real-item duplicate handling is UNCHANGED by this fix
# (it stays bounded only by the cross-reply gifts_given gate, reported separately).
func test_repeated_give_item_applies_once_for_real_items_too():
var gs = GameState.new()
MoveApplier.apply([_m("give_item", ["amulet"]), _m("give_item", ["amulet"]), _m("give_item", ["amulet"])],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.inventory.get("amulet", 0), 3, "non-currency give_item is out of scope for this fix")
assert_eq(gs.inventory.get("amulet", 0), 1, "three tags, one amulet")
func test_repeated_non_currency_accept_item_still_applies_each_time():
func test_repeated_accept_item_applies_once_for_real_items_too():
var gs = GameState.new()
gs.add_item("worn_shortsword", 5)
MoveApplier.apply([_m("accept_item", ["worn_shortsword"]), _m("accept_item", ["worn_shortsword"])],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.inventory.get("worn_shortsword", 0), 3, "non-currency accept_item is out of scope for this fix")
assert_eq(gs.inventory.get("worn_shortsword", 0), 4, "two tags, one sword taken")
func test_different_real_items_in_one_reply_both_apply():
var gs = GameState.new()
MoveApplier.apply([_m("give_item", ["amulet"]), _m("give_item", ["ring"])],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.inventory.get("amulet", 0), 1)
assert_eq(gs.inventory.get("ring", 0), 1)
func test_repeated_disposition_tags_cannot_exceed_the_per_reply_cap():
# MAX_DELTA means "one line cannot swing standing wholesale" (§7 spirit). It was
# clamped PER TAG, so three +15 tags moved standing by 45. The cap is per REPLY.
var gs = GameState.new()
MoveApplier.apply([_m("adjust_disposition", ["+15"]), _m("adjust_disposition", ["+15"]),
_m("adjust_disposition", ["+15"])], gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.npc_dispositions.get("fenn", 0), MoveApplier.MAX_DELTA, "+45 is capped to +15")
func test_disposition_cap_applies_to_the_net_swing():
var gs = GameState.new()
MoveApplier.apply([_m("adjust_disposition", ["+10"]), _m("adjust_disposition", ["+10"])],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.npc_dispositions.get("fenn", 0), 15, "+20 is capped to +15")
func test_disposition_tags_of_opposite_sign_net_out():
# The cap is on the reply's NET swing, not on the tag count — so a walk-back
# inside one reply still lands where the arithmetic says.
var gs = GameState.new()
MoveApplier.apply([_m("adjust_disposition", ["+10"]), _m("adjust_disposition", ["-4"])],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.npc_dispositions.get("fenn", 0), 6)
func test_become_hostile_still_wins_after_a_disposition_tag():
# Moves apply in emission order — the hostile floor must not be undone by
# batching the disposition arithmetic.
var gs = GameState.new()
MoveApplier.apply([_m("adjust_disposition", ["+15"]), _m("become_hostile")],
gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.npc_dispositions.get("fenn", 0), MoveApplier.HOSTILE_FLOOR)
func test_disposition_cap_is_per_reply_not_per_conversation():
# Two separate replies each get a fresh MAX_DELTA budget — standing still moves
# over a conversation, it just cannot lurch in a single line.
var gs = GameState.new()
MoveApplier.apply([_m("adjust_disposition", ["+15"])], gs, CanonLog.new(), _content(), "fenn")
MoveApplier.apply([_m("adjust_disposition", ["+15"])], gs, CanonLog.new(), _content(), "fenn")
assert_eq(gs.npc_dispositions.get("fenn", 0), 30)

View File

@@ -210,10 +210,14 @@ model selecting a subset of a closed, six-element, state-gated set — an NPC ha
a gold, a silver and a copper is a legal sentence — not inventing an integer. Unary
repetition was what made it unbounded, and that is closed.
**The same duplicate-tag path exists for non-currency moves and predates this work**
(three `give_item(amulet)` tags yield three amulets; a repeated `offer_quest` can
double-add a quest). It is bounded *across* replies by `gifts_given` / `has_quest`, but
not *within* one. Deliberately left alone here — it is not a currency bug. **Open.**
**The same duplicate-tag path existed for non-currency moves and predated this work.**
Closed in a follow-up (`fix/duplicate-move-tags`), which generalized the rule — *any*
move that names a thing applies at most once per reply — and so **deleted** the currency
special-case rather than leaving two mechanisms side by side. That follow-up also found
`MAX_DELTA` was clamped **per tag**, so three `[ADJUST_DISPOSITION: +15]` tags moved
standing by 45; the cap is now on the reply's net swing, which is what the constant's own
comment always claimed. (`offer_quest` and `reveal` turned out to be safe already —
`add_quest` self-guards and `mark_revealed` is idempotent.)
**`NpcContent.available_moves`:**