Repository navigation
Delete four surfaces the engine declared and never set - #127
Merged
Merged
Conversation
…urfaces) Acceptance tests marked expected-to-fail: the surrender path, Character.literacy, DungeonState.discovered_features, and the unbreakable ward parameter are removed. Claude-Session: https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs
The surrender path, `Character.literacy`, `DungeonState.discovered_features`, and the `unbreakable` ward param are gone, along with every read site and every sentence that described them. None had a rule behind it, and `schema_version` is already 4 in this release, so the removals ride that bump. Morale prose that quotes the SRD's "flee or surrender" stays; the token `surrendered` is gone from every Python source under `src/`. A schema 3 save may carry `surrendered` on an encounter group or `discovered_features` on the dungeon state. Neither model declares an `extra` policy, so pydantic ignores both keys on load and `_migrate_3_to_4` needs no code for them; its docstring records that. `literacy` was a computed property and never reached a payload at all. No golden moved and no draw sequence changed. Running `tests/generate_phase4_goldens.py` reproduced the stored `phase4_delve.json` apart from the `engine_version` stamp, which the goldens keep, so the file is left as it stands. Beyond the files the packet listed, four more had to change to clear the token and keep the suite running: `crawl/exploration.py` reads the flag when it resolves an area item, `crawl/commands.py`, `crawl/interpreter.py`, and `crawl/triggers.py` name the outcome in docstrings, `core/abilities.py` carried a cross-reference to the removed property that the strict docs build would reject, and `examples/tui_crawler/__main__.py` reads the flag when it picks a target group. `tests/generate_phase4_goldens.py` reads it too, and is edited only through that one clause. `tests/test_character.py::TestCharacterModel::test_literacy_and_languages` asserts on the removed property and is left for the lead to change. Claude-Session: https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs
… interpreter test drops the dead outcome (chunk unwired-surfaces) Claude-Session: https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs
`main` moved the `withdraw` bullet into a new `### Removed` heading and added two bullets under `### Changed`. Both sides are kept: the two new `### Changed` bullets stand, and this chunk's bullet moves to `### Removed` beside the `withdraw` one, where a deletion belongs. The bullet also drops a wrong claim. `MonsterDefeatedEvent.outcome` and `DefeatedMonsterRecord.outcome` are plain strings and always were, so nothing stops a caller from writing any value into one. What changed is that the engine no longer produces the surrender outcome, and the bullet now says that. Claude-Session: https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs
…r path `NpcParty.treasure` said the band's treasure changes hands when they are killed or surrender, a state the engine cannot reach, and now names the two outcomes it can: the band is slain, or it runs away and keeps what it carries. The `battle.declaration.unknown_group` and `items.use.unknown_target` descriptions in `tools/docs/rejection_codes.json`, which the rejection-code reference is generated from, said "fled or surrendered" and now say fled, which is what the two handlers test. The `_migrate_3_to_4` docstring called `literacy` one of three fields and then said it was a property. It now counts the two fields separately from the property. Claude-Session: https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs
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.
Part of #100. The
check_fatiguerow of that issue lands with #124, so the issue closes when both are merged.The docstring pass turned up five things the code declared and nothing ever set. The project's rule is to wire a surface or delete it, none of these has an SRD rule behind it, and
schema_versionis already 4 in this release, so the removals ride that bump.What a consumer no longer sees:
EncounterGroup.surrendered, thebattle.side.surrenderedcode onMonsterFledEventand its default English template, and the"surrendered"defeat outcome.MonsterDefeatedEvent.outcomeandDefeatedMonsterRecord.outcomeare plain strings and stay that way; what changed is that the engine no longer produces that value, so the outcome it reports is"slain"or"routed".Character.literacy. The band still comes fromAbilityTables.literacy, given the character's intelligence score.DungeonState.discovered_features.unbreakableward param, so melee against a barred monster breaks every party ward.Morale prose that quotes the SRD's "flee or surrender" stays. The token
surrenderedis gone from every Python source undersrc/, which is what the first acceptance test scans for.Old saves: a schema 3 payload may carry
surrenderedon an encounter group ordiscovered_featureson the dungeon state. Neither model declares anextrapolicy, so pydantic drops both keys on load and_migrate_3_to_4needs no code for either; the migration's docstring records that.literacywas a computed property and never reached a payload at all.Nothing moved that determinism governs. Running
tests/generate_phase4_goldens.pyreproduced the storedphase4_delve.jsonapart from theengine_versionstamp the goldens keep, so no golden is touched.Verification, from the worktree:
uv run pytest --runxfail tests/test_unwired_surfaces.py tests/test_interpreter.py: 61 passed.uv run ruff format --check,uv run ruff check,uv run pyright,uv run mkdocs build --strict: all clean.uv run pytest: 2886 passed, 146 skipped, 3 xpassed. The three xpassed are this chunk's acceptance tests, whose xfail markers are left for the lead to remove at merge.Review corrections are in. The changelog bullet moved under the
### Removedheadingmainnow has, withorigin/mainmerged in and both sides of the conflict kept, and it no longer claims the twooutcomefields refuse the value.NpcParty.treasureno longer says the band's treasure changes hands when they surrender. Thebattle.declaration.unknown_groupanditems.use.unknown_targetdescriptions intools/docs/rejection_codes.jsonsay fled. The_migrate_3_to_4docstring counts two fields and one property rather than three fields.Eight files outside the packet's list changed.
core/npc.pyandtools/docs/rejection_codes.jsonare the review's own corrections.crawl/exploration.pyreads the flag when it resolves an area item andexamples/tui_crawler/__main__.pyreads it when it picks a target group, so both would have broken.crawl/commands.py,crawl/interpreter.py, andcrawl/triggers.pyname the removed outcome in docstrings, which the token scan covers.core/abilities.pycarried a cross-reference to the removed property that the strict docs build would reject.tests/generate_phase4_goldens.pyreads the flag in one filter clause and is edited only there.https://claude.ai/code/session_01NmCezTw8hKKujkaEZ3YGAs