Skip to content

docs(oracle-parser): document the two parse_type_phrase grammars - #7401

Draft
JacobWoodson wants to merge 1 commit into
phase-rs:mainfrom
JacobWoodson:docs/oracle-parser-two-type-phrase-grammars
Draft

docs(oracle-parser): document the two parse_type_phrase grammars#7401
JacobWoodson wants to merge 1 commit into
phase-rs:mainfrom
JacobWoodson:docs/oracle-parser-two-type-phrase-grammars

Conversation

@JacobWoodson

Copy link
Copy Markdown
Contributor

Summary

Splits the .claude/skills/oracle-parser/SKILL.md change out of #7396, per the
maintainer review on that PR: .agents/pr-review-policy.toml classifies
.claude/skills/** as a [hard_stops] path, so agent-instruction changes must
not ride along with an implementation PR.

#7396 has been force-pushed with this hunk removed and is now hard-stop clear.

What this documents

oracle_nom::target::parse_type_phrase and oracle_target::parse_type_phrase
are two different readers with the same name, and picking the wrong one silently
changes which cards a characteristic head accepts:

Strictoracle_nom::target Legacyoracle_target
Return OracleResult<'_, TargetFilter> = (remainder, filter) (TargetFilter, &str) = (filter, remainder)
Failure Err Infallible — empty TypedFilter + whole input (not TargetFilter::Any, so an Any guard does not catch it)
Type-list join " or " only " or ", " and ", " and/or " and comma forms
Ownership / token / combat-relation grammar no yes

Both signatures verified against current main:
crates/engine/src/parser/oracle_nom/target.rs:41 and
crates/engine/src/parser/oracle_target.rs:2135.

Ordering constraint

Merge this after #7396. The closing paragraph references
TypePhraseGrammar, the typed parameter #7396 introduces in
oracle_nom/quantity.rs. That symbol does not exist on main today
(git grep TypePhraseGrammar origin/main -- crates/ is empty), so landing this
first would leave a dangling reference. Opened as a draft for that reason.

Verification

Documentation only — no code, no build surface. The two-grammar facts in the
table were re-read against origin/main rather than carried over from #7396.

Split out of phase-rs#7396 per the maintainer hard-stop policy on ".claude/skills/**"
(.agents/pr-review-policy.toml): agent-instruction changes must not ride along
with an implementation PR.

oracle_nom::target::parse_type_phrase and oracle_target::parse_type_phrase have
swapped return shapes and different grammars, so importing the wrong one
silently changes which cards a characteristic head accepts. Documents both,
their failure modes (Legacy is infallible and yields an empty TypedFilter, not
TargetFilter::Any), and their differing type-list join separators.

Note: the closing paragraph references TypePhraseGrammar, which is introduced
by phase-rs#7396 - merge this after that PR lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 62382fcd-0372-4150-adc1-0aacb9fb6c11

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matthewevans matthewevans self-assigned this Aug 14, 2026
@matthewevans matthewevans added the refactor Refactor label Aug 14, 2026

@matthewevans matthewevans 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.

Changes requested — explicit human maintainer review required.

At current head 9710204f5c4c3980b209f665eb27dddbbbe4f610, this external-contributor PR changes protected instruction path .claude/skills/oracle-parser/SKILL.md. Repository policy classifies .claude/skills/** as a hard-stop path. I have not implementation-reviewed or approved this change. Please obtain an explicit human maintainer review before any further automated handling.

@matthewevans matthewevans removed their assignment Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants