Skip to content

validate switch cases against mapper values - #22

Open
VasilisDragon wants to merge 2 commits into
ProtoDef-io:masterfrom
VasilisDragon:fix-mapper-switch-validation
Open

VasilisDragon wants to merge 2 commits into
ProtoDef-io:masterfrom
VasilisDragon:fix-mapper-switch-validation

Conversation

@VasilisDragon

@VasilisDragon VasilisDragon commented Sep 12, 2026 •

Copy link
Copy Markdown

Reject literal named switch cases that aren’t outputs of a known mapper, following ordinary container paths and unambiguous aliases. Leave unknown semantics and numeric cases unchecked where the engines differ.

Adds generic regression controls. Package tests pass on Node 24.15.0; private checks also pass the unchanged ProtoDef suite and all 113 physical PC/Bedrock protocol definitions.

Fixes #21.

@VasilisDragon

Copy link
Copy Markdown
Author

Found one edge here: mapper output "1" with switch case "0x1" round-trips in the compiler, but the interpreter and this check reject it. I’d leave ambiguous numeric matches unchecked while keeping the named-case checks. Does that fit what you had in mind?

@rom1504 rom1504 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Astra agent review — AI-generated, not manually written by the maintainer.

The generic mapper/switch check belongs in this validator and the opaque-scope skips are appropriate, but the numeric-key ambiguity already identified here still needs handling. Mapper output "1" can match switch key "0x1" in the supported compiler while this literal-string membership check rejects it.

Leave ambiguous numeric relationships unchecked until the engine contract is unified, while retaining strict validation of semantic named cases. Add that control to the generic tests. I have no additional root cause to duplicate inline and did not rerun the full protocol matrix.

Skills used: prismarine-review checked the current revision and feedback; prismarine-protocol-data-review checked codec/schema semantics; prismarine-architecture-review checked the corresponding consumer API.

@VasilisDragon

Copy link
Copy Markdown
Author

Left numeric cases unchecked while keeping named-case validation, including mixed switches. Added generic controls for that boundary.

I also verified the raw-ID passthrough added in ProtoDef #183. The package tests and unchanged ProtoDef suite pass, and private validation accepts all 113 physical PC/Bedrock protocol definitions without input mutation. Numeric acceptance remains an exclusion, not a guarantee of engine parity.

@VasilisDragon

Copy link
Copy Markdown
Author

Could a maintainer approve the pending CI run for the latest revision? GitHub is waiting for workflow approval, so the jobs haven't started yet. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add protocol mapper sanity check to protodef-validator

2 participants