Skip to content

feat(rules): convert a rule-content-v1 statement into the public model - #534

Merged
e54-bot merged 3 commits into
mainfrom
claude/statement-from-content-a8e1ef
Oct 20, 2026
Merged

e54-bot merged 3 commits into
mainfrom
claude/statement-from-content-a8e1ef

Conversation

@e54-bot

@e54-bot e54-bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #531

Adds rules::statement::statement_from_content(catalog, node), the input direction of rule-content-v1. It converts one condition ({"value": node}) or action ({"call", "args"}) list item, with optional "disabled": true, into a Statement. A node from Rule::content is valid input unchanged. The format id and schema are unchanged.

StatementContentError has distinct variants, each naming the node by JSON pointer: UnknownId, ArgumentCount, InternalSentinel, Unrepresentable, Spans. Argument counts follow the same rule canonical validation applies. An ambiguous enum node and an opaque rawWorkshopAction are refused as Unrepresentable.

Verification:

  • content(statement_from_content(content(s))) == content(s) over the practice-range exports and the corpus fixtures. The practice-range files are parsed in en-US and zh-CN, and the program is emitted and re-parsed in every locale that can render it (emission skips locales missing a spelling; en-US and zh-CN are asserted). Round-trip is verified only for en-US and zh-CN because the catalog lacks spellings in the other 14 locales; statement content is locale-independent.
  • Refusal tests for each error variant, ambiguous enums, nested spans and disabled modifiers.
  • fmt, clippy -D warnings, full tests, workshop-catalog-gen check on stable and 1.88.0.
  • cargo semver-checks reports two failures (SettingsListElement.variant, LookupMatch::Builtin.description). Both are in code this PR does not touch, and no failure names the new module.

Argument-count checking is one shared CatalogEntry::rejected_arity used by canonical validation and by conversion, with an agreement test over a folded player-variable pair, a variadic entry and an undeclared-signature entry.

Not in scope: emission and edits (#532).

Add rules::statement::statement_from_content. It turns one condition or action list item into a Statement, and refuses unknown ids, wrong argument counts, internal sentinels, unrepresentable nodes (including ambiguous enum members) and spans with distinct structured errors. The rule-content-v1 format id and schema are unchanged.

Fixes #531

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One defect against the repo's contract rules; everything else I checked holds.

Arity check reimplements the established path instead of sharing it (crates/workshop-rs/src/rules/statement.rs, Converter::check_arity). It copies the rule in values::validate::validate_call_signature (undeclared-signature skip, folds_player_variable_pair +1, required_param_count..param_count, variadic). workshop-rs/AGENTS.md "Sibling paths share checks" requires reusing the established path's checks and adding a test that both paths agree on the same inputs. The PR body states the rule is the same, but nothing enforces it: when validation changes (for example a new fold case), conversion silently diverges and accepts or refuses differently from canonical validation. No agreement test exists.

Required correction, smallest level: extract the count rule into one CatalogEntry helper taking the supplied count (and the fold flag), call it from both validate_call_signature and check_arity, and add one test that feeds the same call (including a folded player-variable pair, a variadic and an undeclared-signature entry) through conversion and canonical validation and asserts both accept or both refuse.

Checked and fine: fmt, clippy -D warnings, full workspace tests and catalog check pass locally; CI is green (stable, 1.88.0, public API, catalog, real-project). Round-trip passes over practice-range (en-US, zh-CN) and corpus fixtures; 5 distinct error variants with JSON pointers; spans (top-level and nested) rejected; ambiguous enum and raw action refused; additive API; docs updated; no emission or edit code (#532).

…nd statement conversion

Extract CatalogEntry::rejected_arity so canonical validation and statement_from_content apply one rule, and add an agreement test over a folded player-variable pair, a variadic entry and an undeclared-signature entry.

@Teakowa Teakowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM: the argument-count rule now lives once in CatalogEntry::rejected_arity and both validate_call_signature and statement_from_content call it. The new agreement test checks accept and reject for every count on a folded player-variable pair, a variadic entry and an undeclared-signature entry. fmt, clippy -D warnings and the full workspace tests pass in a fresh worktree on 99d1ec4, scope is unchanged, and all 5 CI checks are green.

@e54-bot
e54-bot merged commit fc7b883 into main Oct 20, 2026
10 checks passed
@e54-bot
e54-bot deleted the claude/statement-from-content-a8e1ef branch October 20, 2026 09:44
Teakowa added a commit that referenced this pull request Oct 20, 2026
Record the shared-authority class from the #534 review finding: statement_from_content re-derived the arity rule validate_call_signature owns. Same class as the sibling-surface divergences tracked in #451.
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.

Convert a rule-content-v1 statement into the public model

2 participants