Skip to content

fix(settings): reconcile raw projections - #122

Merged
Teakowa merged 4 commits into
mainfrom
codex/issue-110-projection-reconciliation
Aug 28, 2026
Merged

fix(settings): reconcile raw projections#122
Teakowa merged 4 commits into
mainfrom
codex/issue-110-projection-reconciliation

Conversation

@e54-bot

@e54-bot e54-bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reconcile generated entries that conflicted with fixture-owned rendering or enum identities
  • reject full-contract raw projection conflicts instead of hiding them through lookup precedence
  • detect conflicting or orphaned enum projections

Verification

  • cargo fmt --all --check
  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo run -p workshop-rs --bin workshop-catalog-gen -- check
  • git diff --check

Refs #110

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

Full QA review — fb52abf7281fd4454b3d738d1ca705b71c36b25b

Blocker

  1. The PR resolves projection conflicts by deleting reviewed export enum evidence instead of reconciling it into the canonical domains.

The full-contract raw-entry comparison is a correct improvement, but the data remediation removes several generated enum domains/members without migrating their non-overlapping members to the fixture-owned canonical domains. Examples removed by this PR include:

  • heroLimit: 1PerTeam, 2PerTeam, 1PerGame, 2PerGame;
  • roleLimit: off;
  • spawnHealthPacks: modeDependent, enabled;
  • mapRotation: afterMirrorMatch, paused;
  • returnToLobby: afterMirrorMatch.

The fixture-owned domains retained on this head are substantially narrower (for example heroLimit retains only off; spawnHealthPacks only disabled; mapRotation only afterAGame). These deleted members came from the reviewed Workshop-data export projection; removing them makes the validator green by discarding evidence rather than reconciling the two sources.

This is observable in the typed API: enum writes call table::enum_name(domain, member) and reject members that are absent from the canonical enum tables, so the removed members become invalid after this PR. That conflicts directly with #110's requirement to reconcile reviewed settings facts into one canonical dataset and to remove/reroute duplicate semantic tables only after the canonical catalog is equivalent or more accurate.

Please map the export-backed members into the chosen canonical enum domains/identities (with explicit provenance/reconciliation) rather than dropping them. Add a representative regression using a real affected domain so export-only members cannot disappear while workshop-catalog-gen check still passes.

Major

  1. Enum conflict validation is only one-directional. validate_enum_projection() detects the same (domain, member) mapping to two different display names, but it does not detect the inverse conflict: the same (domain, display name) mapped to different canonical member IDs. The data being reconciled already demonstrates this shape: fixture mapRotation.afterAGame -> "After A Game" versus generated mapRotation.afterGame -> "After A Game" (similarly for returnToLobby). Once the domains are correctly unified instead of deleted, this ambiguity would pass the current validator. Canonical enum member identity needs an explicit reconciliation rule or a reverse-collision check.

  2. The reconciliation is not currently encoded in a reproducible generation path. The PR edits data/generated_entries.rs directly and changes its header to say certain fixture-owned conflicts are “reconciled out”, but no generator/source rule is changed that records which export entries/members are excluded or remapped. workshop-catalog-gen validates the committed settings projection but does not regenerate this file. #110 explicitly requires a deterministic canonical dataset/generation path; the reconciliation decision should live in durable source/generation data rather than only in a hand-edited generated artifact, otherwise a future export refresh can reintroduce the same conflict.

Verified improvements

  • raw duplicate paths now compare the full TableEntry contract, so enum-domain and presentation differences are no longer hidden by first-wins lookup;
  • projection diagnostics identify fixture vs Workshop-data export sources;
  • orphaned enum domains and same-member/different-name conflicts are checked independently of lookup order;
  • stable/MSRV Rust quality, Catalog check, and all five real-project scenarios are green on this exact head.

The PR is not ready to merge while the Blocker remains; #110 should stay open.

@e54-bot

e54-bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 5fa213b.

  • Restored the complete reviewed export projection and mapped all affected export enum members into the fixture-owned canonical domains instead of deleting them.
  • Added projection_reconciliation.json, validated by workshop-catalog-gen check, which pins each divergent raw entry contract and every export-to-canonical enum member mapping.
  • Enum validation now rejects both same-member/different-name and same-domain/display-name/different-canonical-member conflicts after reconciliation.
  • Added a typed heroLimit: 1PerTeam write/emission regression to preserve export-only members.

Verified: cargo test --workspace --all-targets (264 passed), clippy, catalog check, and diff check.

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

Re-review — exact head 50b7a45b6df22b0d534eebd58b8c869728ae3770

The previous Blocker is resolved: the reviewed Workshop-data export projection is restored, export-only enum members are retained through explicit canonical mappings, reverse display-name/identity collisions are checked, and the typed heroLimit / mapRotation regressions demonstrate observable preservation. The reconciliation manifest also solves the previous reproducibility concern without editing the generated export artifact.

Major

  1. The reconciliation manifest is not itself conflict-checked, so contradictory canonical mapping rules can still pass workshop-catalog-gen check.

validate_enum_projection() finds a mapping with mappings.iter().find(...) and tracks only whether a (sourceDomain, sourceMember) was consumed. If projection_reconciliation.json contains two mappings for the same source member with different targets, the first mapping wins; the second is then considered satisfied by the same mapped_sources key. No error is emitted. entryOverrides has the same shape: reconciled_entry_override() uses .find() by path, so duplicate rules for one path can silently shadow each other.

That leaves the new durable reconciliation source with the same first-wins class of failure this PR is intended to remove from the raw tables. Before #110 can treat this as a conflict-checked canonical generation path, validate the reconciliation data itself (at minimum unique entryOverrides.path and unique (sourceDomain, sourceMember) mappings; conflicting duplicate rules must fail). A small mutation/unit test for duplicate source mappings and duplicate path overrides would directly protect the contract.

Minor

  1. The canonical enum provenance comment is now factually stale. table.rs still says roleLimit has exactly one evidenced member and that off is rejected until a snapshot evidences it, while this PR intentionally accepts roleLimit.off from the reviewed export and adds several other export-backed members to ENUM_MEMBERS. Update the comment so it distinguishes fixture evidence from export-backed canonical members instead of documenting behavior that is now false.

  2. projection_reconciliation.json declares schemaVersion: 1, but Reconciliation does not deserialize or validate that field. If the version is meant to be a durable data-contract marker, parse/check it (and preferably reject unsupported versions); otherwise remove the unused field rather than implying version enforcement.

Verified

  • previous export-evidence loss is fixed;
  • afterGame -> afterAGame and other explicit identity mappings are represented durably;
  • full raw TableEntry contract comparison remains intact;
  • same-member/different-name and same-display/different-member conflicts are covered;
  • stale enum mappings whose source disappears are rejected;
  • exact-head CI is green across stable/MSRV Rust quality, Catalog check, and all five real-project scenarios.

No additional Blocker/Major findings were found across the current PR scope. The PR should stay open until the manifest self-conflict check is closed.

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

Final QA — exact head 8284a2a0d2e55718c3ffd1c5681dfc431f75ee4c

0 Blocker / 0 Major / 0 Minor.

The remaining reconciliation-manifest Major is resolved: the manifest now rejects duplicate entry override paths and duplicate enum mapping source identities before first-match lookup can be used, and unsupported schemaVersion values fail validation. The catalog check invokes this validation directly. The stale enum provenance comment was also corrected.

Previously resolved guarantees remain intact: reviewed export enum evidence is preserved, source identities are explicitly mapped into canonical domains, reverse display-name/member conflicts are checked, stale mappings are rejected, and representative typed heroLimit / mapRotation writes remain covered.

Exact-head CI is green for stable Rust quality, Rust 1.85 quality, Catalog check, and all five real-project scenarios.

This head is ready to merge for the scoped #110 projection-reconciliation fix.

@Teakowa
Teakowa merged commit 089669b into main Aug 28, 2026
4 checks passed
@Teakowa
Teakowa deleted the codex/issue-110-projection-reconciliation branch August 28, 2026 19:01
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.

2 participants