Conversation
…jections #109 Closed three CRITICAL bypasses introduced by #97's single-sub-roll Group passthrough at `parseCritThreshold`. All three shared one root cause — shallow reject helpers that the new Group route walked past while `containsDicePool`'s deep walk pulled the inner pool through. Added `containsMultiSubGroup` deep-walk to `ast.ts` and used it from `rejectGroupTarget` so a buried multi-sub Group (`{{1d20, 1d20}kh1}cs>18`, `{abs({1d6, 2d8})}cs>5`, `{-{1d6, 2d8}}cs>5`, `{floor({1d6, 2d8}/1)}cs>5`) re-rejects with `INVALID_CRIT_THRESHOLD_TARGET`. Added `deepContainsFatePool` mirroring `deepContainsDicePool` and used it from `containsFatePool`'s Group case so `{4dF+1d6}cf`, `({4dF+1d6})cf`, and `{abs(4dF)}cf` no longer flip Fate `+1` faces to `fumble: true` via the bare-cf default check. Added `containsVersus` deep-walk and used it from `rejectVersusTarget` so Versus buried under arithmetic/function-call inside a single-sub Group (`{1d20 vs 15}cs>18`, `{1+(1d20 vs 15)}cs>18`, `{abs(1d20 vs 15)}cs>18`, `{1d20 vs 15}s`, `4d6>={1d20 vs 15}`) throws `NESTED_VERSUS` instead of silently dropping `degree`/`natural`. Added the missing `rejectVersusTarget` call to `parseModifier` so `{1d20 vs 15}kh1` rejects too — closes a pre-existing inconsistency where `(1d20 vs 15)kh1` rejected with `INVALID_MODIFIER_TARGET` while the brace form silently dropped metadata. Strengthened the evaluator round-trip test (asserts `result.expression === '{1d20}kh1cs>18'` and tightened the MockRNG sequence to a single value) and added evaluator coverage for the four flipped accept tests (`{1d6}cs>5`, `({1d6})cs>5`, `{1d6}cf<2`, `{1d6}kh1cf<2`). Updated `STAGE3.md` group-edge-cases list and `CHANGELOG.md [Unreleased]` with three entries covering the closures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closed three CRITICAL bypasses introduced by #97's single-sub-roll Group passthrough. All three shared one root cause: shallow reject helpers vs. the deep
containsDicePool/deepContainsDicePoolwalk that the Group passthrough now relies on.containsMultiSubGroupdeep-walk toast.ts;rejectGroupTargetuses it on the single sub-expression so a multi-sub Group buried under arithmetic, function calls, or unary ops re-rejects ({{1d20, 1d20}kh1}cs>18,{abs({1d6, 2d8})}cs>5,{-{1d6, 2d8}}cs>5,{floor({1d6, 2d8}/1)}cs>5).deepContainsFatePoolmirroringdeepContainsDicePool;containsFatePool's Group case switches to it so{4dF+1d6}cf,({4dF+1d6})cf, and{abs(4dF)}cfno longer flip Fate+1faces tofumble: true.containsVersusdeep-walk;rejectVersusTargetuses it againstGroup([single])'s inner expression so Versus buried under arithmetic/function-call rejects withNESTED_VERSUS({1d20 vs 15}cs>18,{1+(1d20 vs 15)}cs>18,{abs(1d20 vs 15)}cs>18,{1d20 vs 15}s,4d6>={1d20 vs 15}).rejectVersusTargetcall toparseModifierso{1d20 vs 15}kh1rejects — closes a pre-existing inconsistency where the parens form rejected withINVALID_MODIFIER_TARGETwhile the brace form silently droppeddegree/natural.result.expression === '{1d20}kh1cs>18'(preserves user notation) and tightened the MockRNG sequence to one value (was four — three were dead weight).{1d6}cs>5,({1d6})cs>5,{1d6}cf<2,{1d6}kh1cf<2).{1d20 vs 15}+5continues to ACCEPT —BinaryOpusesmergeContextand propagatesdegree/naturalcorrectly. Explicitly tested.STAGE3.mdgroup edge-cases list andCHANGELOG.md [Unreleased]with three Fixed entries.Closes #109
Drafted with AI assistance