Repository navigation
W3-9: replace Any in content_loader, type query_handler payloads - #87
Merged
Merged
Conversation
…loads content_loader: _read_yaml/_load_section now return dict[str, object]; nested YAML values are narrowed with new utils helpers (as_mapping, as_list, as_mapping_list, as_int) that raise ValueError naming the offending file/key. Parse entry points take Mapping[str, object]; pydantic schema dict fields use object (JSON Schema output unchanged). query_handler: EntitySummary, EntityDetail, NpcDetail, CombatInfo and their nested TypedDicts replace dict[str, object] for fixed-shape query answers. Wire output is unchanged (same keys, same insertion order).
Owner
Author
|
Independent review: PASS
|
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.
Card W3-9 (issue ff6cb95c128b1a6b57cb:
any-to-object-sweep,dict-str-object-overuse). Based on main 2f79530. I checked both items against main before starting and neither was already fixed.Root cause
content_loader/utils._read_yamlreturneddict[str, Any], so every YAML value in the loader wasAnyand mypy checked none of the reads.query_handler.pybuilt fixed-shape query answers as plaindict[str, object].Fix
content_loader (0
Anyleft, was 43)_read_yaml/_load_sectionnow returndict[str, object]. A document whose top level is not a mapping now raisesValueError.utils.pynarrow nested YAML values and raiseValueErrorthat names the file or key:as_mapping,as_list,as_mapping_list,as_int.as_intaccepts the same scalar inputs thatint()did.parse_npc,parse_player,parse_squad,parse_monster_template,parse_encounters,parse_lairs,deserialize_item, …) now takeMapping[str, object].parse_itemstakesSequence[Mapping[str, object]], andparse_attackstakesSequence[object].class_featuresandmodifiersnow useobject. I compared the dumped JSON Schemas of every registry entity type and every schema class before and after: they are byte-identical.schema_gen.get_entity_schema→dict[str, object], with isinstance narrowing before it injectsx-ref-type.manifest.py,assembly.pyandservice/commands_worldbuilder.pynarrowlayers/layers.<type>withas_mapping.commands_worldbuilder.pyneeded this because_read_yamlnow returnsobjectvalues.query_handler
dict[str, object]return types:EntitySummary,EntityDetail(kind-specific keys areNotRequired),NpcDetail,CombatInfo, plus the nestedEquippedWeaponInfo,InventoryEntryInfo,ResourcePoolInfoandActivationTriggerInfo.params: dict[str, object]stay as they are: their shape differs per query type, so they are not a fixed shape.No new
type: ignores were added.mypy --strictpasses.Behaviour changes (bad content only)
Valid content behaves exactly as before. Malformed content now fails earlier, with a
ValueErrorthat names the location:AttributeError.tags: medieval(a string instead of a list) in librarymetadata.yamlused to turn silently into['m','e','d',…]. This was a small real bug.list(str).as_listnarrowing.test_library_tags_must_be_a_list.class_featuresvalue that is not a mapping used to raiseAttributeError.ValidationError, which is itself aValueError. It now raises a plainValueErrorwith the location in the message.ref:/base:value used to raiseTypeError. It now raises the existing "unknown catalog entry"RuntimeError.tags/requires_geographyentries are nowstr()-coerced, which matches the declaredlist[str].Tests
New tests in
tests/unit/test_content_utils.pycover the narrowing helpers and the new error paths. I ran these 4 against main'ssrcand confirmed they fail on main:TestYamlHelpers::test_non_mapping_top_level_is_rejectedTestBadContentIsNamed::test_library_tags_must_be_a_listTestBadContentIsNamed::test_npc_entry_must_be_a_mappingTestBadContentIsNamed::test_class_features_block_must_be_a_mappingThe
TestNarrowinghelper tests test new code, so they fail on main only because the helpers cannot be imported there.make checkis green: 2957 backend tests pass, 353 frontend tests pass, and ruff and mypy are clean.docker-compose.test.ymlsettings (DND_DICE_SEED=42,DND_CONTENT_DIR=tests/integration/content) and ranBACKEND_URL=http://localhost:8001 pytest tests/integration/. 184 passed.Found but not fixed (out of scope / needs a decision)
core/models.Query.paramsis stilldict[str, Any], andInnerSelf.to_dictreturnsdict[str, Any]. Both are outsidecontent_loader.creatures.pykeeps its two existing# type: ignore[arg-type]oncombat_position=tuple(...). Removing them needs a typedtuple[int, int]conversion in the schema, which is a design choice.grant_modifiersentries onItemContentare still untyped dicts, and the manifest is read as raw YAML in 4 places (manifest.py,assembly.py×2,commands_worldbuilder.py). Proper pydantic models (ModifierContent,ManifestContent) would give real validation, but would change the published JSON Schema and error types. I left that for a design decision.