Phase 14 plan — triggers and the interpreter - #58
Merged
Merged
Conversation
The review found one blocking hole: the plan claimed live-with-interpreter
final state equals listener-free replay state byte-for-byte, but
register_listener itself creates the interpreter's (empty) listener_state
entry, which a listener-free replay never has — the phase 5 milestone
golden already compares modulo listener_state for exactly this reason.
The comparison is now pinned: equality on every block except
listener_state, with the interpreter's entry asserted to be exactly {},
and command/event logs byte-equal.
Non-blocking fixes: the spec amendment now carries the consequence-surface
exclusions (IdentifyItem, RollDice, the lifecycle family) alongside the
selectors, per the phase 12 altitude test; the census cross-check says two
named exclusions, not three; and phase 13's handed-off decision on
MarkTriggerFired.trigger_id validation is answered on the record (stays
open, SetFlag's key precedent).
Claude-Session: https://claude.ai/code/session_01AH3H26T16968mmY5e4ZDJB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementation plan for phase 14 of the spec: the trigger spec, authored triggers on the adventure document, and the library-shipped interpreter listener — document-order matching, once-only fired-state, the depth-4 cascade bound with truncation notes, and dropped-consequence recording;
validate_adventuregrows trigger reference checks. Milestone: the lever-opens-portcullis scenario authored as data replays identically with no listeners, and a trigger spawn colliding with an open encounter drops and records its note.Notable decisions
session.execute()insidehandle()is the proven issuance channel (FetchQuestListener), but nested-command events never reach the outerCommandResult(a front end would render nothing of the portcullis opening), and_persist_sightruns after the listener loop (a relocating consequence would make the seen map diverge under replay). The plan fixes both at the seam: the listener-loop splice and the sight-persistence reorder — no stored golden changes, since goldens snapshot state and logs, never result envelopes.([], {})from everyhandle()and keeps no state the engine can't reconstruct; the phase golden asserts live-vs-listener-free-replay equality on every state block exceptlistener_state(whose interpreter entry is asserted to be exactly{}— the phase 5 modulo-listener_stateprecedent, strengthened), with command and event logs byte-equal.MarkTriggerFired/TriggerFiredEventat referee visibility via the phase 11 event-beat mechanism; a trigger's player-facing voice is its journal form.character_idin an authored consequence must be@partyor@first, expanded by the interpreter to living members at issue time — the log stays concrete and replays exactly. The consequence surface itself is a typed sub-union (ConsequenceCommand, nine classes) so lifecycle and player commands fail at parse; the exclusions plus selectors land as a two-sentence spec amendment with the implementation PR.crawl/triggers.py,crawl/interpreter.py), three additive fields (Adventure.triggersaside), and the seam work.Review provenance
Drafted, then rubber-ducked by a skeptical reviewer with the spec, phases 10–13 plans, and the code as evidence. The review found one blocking hole and three non-blocking items, all accepted on the merits:
register_listeneritself creates the interpreter's emptylistener_stateentry a listener-free replay never has — the phase 5 golden compares modulolistener_statefor exactly this reason. Fixed by pinning the comparison shape (above).RollDice/IdentifyItemfrom the spec's current sentence); a census cross-check miscounted its exclusions; and phase 13's handed-off decision onMarkTriggerFired.trigger_idvalidation needed an on-record answer (stays open,SetFlag's key-domain precedent).The re-review verified each fix against the code and returned SOLID, with one cosmetic naming remnant folded in as a sign-off tweak. The reviewer's verified-good list confirmed, among others: both seam defects are real; the event-log interleaving argument holds against the existing phase 5 byte-for-byte replay test; the collision beat works (
SpawnMonstersmode-legal inencounterbut rejectedencounter_in_progresswhile mark/note/journal land); and the greenfield sweep found no shims, dual paths, or unconsumed scaffolding.https://claude.ai/code/session_01AH3H26T16968mmY5e4ZDJB