[wheels-bot]

Summary

For the struct form of enum() — e.g. enum(property="priority", values={low: 0, medium: 1, high: 2}) — the auto-registered inclusion validation was checking the property against the NAME keys ("low,medium,high") while the auto-generated scopes and is<Name>() checkers compared against the STORED values (0/1/2). No value a row could hold satisfied both sides: saving the stored value tripped validation, saving the name skipped every scope and checker. This was masked in the suite because the only enum fixture (PostWithEnum.cfc) used the list form, where names and values are identical.

The fix builds the inclusion list from the stored values (iterating local.enumDef.values by name), so validation now accepts the same values the scopes filter on and the is*() checkers compare against. The list form path is unaffected because names == values there.

Related Issue

Refs #3014

Type of Change

• Bug fix
• New feature
• Enhancement to existing feature
• Documentation update
• Refactoring

Feature Completeness Checklist

• DCO sign-off -- Every commit carries Signed-off-by: (use git commit -s); see CONTRIBUTING.md
• Tests -- New vendor/wheels/tests/specs/model/structEnumValidationSpec.cfc covers the struct form across validation, scope, and is<Name>() agreement; includes a list-form regression guard. Failed pre-fix (3 of 5 expectations), passes post-fix.
• Framework Docs -- bot-update-docs.yml will follow up if needed
• AI Reference Docs -- bot-update-docs.yml will follow up if needed
• CLAUDE.md -- bot-update-docs.yml will follow up if needed
• Changelog fragment -- changelog.d/struct-enum-validation-mismatch.fixed.md
• Test runner passes -- wheels.tests.specs.model directory: 888 pass / 0 fail / 0 error post-fix (vs. 885 pass / 3 fail / 0 error pre-fix); same 175 suites.

Test Plan

1. bash tools/test-local.sh model — confirm 888/0/0.
2. Inspect the new spec at vendor/wheels/tests/specs/model/structEnumValidationSpec.cfc — five expectations covering:
   • struct-form: valid() accepts the stored value (0)
   • struct-form: valid() rejects neither-name-nor-value ("bogus")
   • struct-form: the value that passes validation is the value is<Name>() returns true for
   • struct-form: the value that passes validation is the value the auto-generated scope queries on
   • list-form regression: names == values continues to validate correctly
3. Sanity-check the existing list-form coverage (vendor/wheels/tests/specs/model/enumSpec.cfc) still green.

Screenshots / Output

n/a

[wheels-bot]

Wheels Bot — No doc updates

Reviewed this PR's diff and found no docs that need updating (bug fix restoring behavior the documentation already described — the MDX enum guide already promised that valid() accepts the stored values and the struct form stores integers; no documented behavior changed, only the broken implementation was corrected).

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: The logic fix is correct — building the inclusion list from stored values rather than name keys is exactly right, and the new spec catches the regression from the other direction. One meaningful test gap needs to be closed before merge: the spec never asserts that the enum name string (e.g. "low") is now rejected for the struct form, which was the specific invariant this PR inverts. There is also a multi-line comment block that violates the project style rule. Both should be addressed before marking ready for review.

---

Tests

Missing assertion: struct-form enum NAME is now rejected

vendor/wheels/tests/specs/model/structEnumValidationSpec.cfc, lines 33–40

it("rejects values that are neither names nor stored values", () => {
    var m = g.model("author")
    m.enum(property="lastName", values={low: 0, medium: 1, high: 2})

    var instance = m.new(firstName="Test")
    instance.lastName = "bogus"
    expect(instance.valid()).toBeFalse()
})

This test only verifies that an arbitrary unrelated string ("bogus") fails validation. It does not verify that the enum NAME itself ("low") is now rejected — yet that is literally the behavior the fix inverts. Before the fix, validatesInclusionOf accepted "low" (from StructKeyList) and rejected 0. After the fix it must accept 0 and reject "low". The "accepts 0" half is covered by tests 1, 3, and 4. The "rejects "low"" half is nowhere in the suite.

Without this assertion, a future regression to the old code path would pass all five tests while silently re-introducing the bug.

Suggested addition:

it("rejects the enum NAME string (struct form) — stored value is what matters", () => {
    var m = g.model("author")
    m.enum(property="lastName", values={low: 0, medium: 1, high: 2})

    var instance = m.new(firstName="Test")
    instance.lastName = "low"  // the name key, not the stored value
    expect(instance.valid()).toBeFalse()
})

---

Conventions

Multi-line comment block in production code

vendor/wheels/model/properties.cfc, lines 919–926

// Auto-register inclusion validation for this property.
// The list must derive from the STORED VALUES (not the name keys), so that
// rows hold values the auto-generated scope filters on and the is<Name>()
// checkers compare against. For the list form, names == values so the
// behaviour is unchanged. For the struct form `enum(property="priority",
// values={low: 0, medium: 1, high: 2})`, validation now accepts 0/1/2 —
// the same values the scope WHERE clause and isLow()/isMedium()/isHigh()
// already reference. See issue ##3014.

CLAUDE.md §Tone: "Never write multi-paragraph docstrings or multi-line comment blocks — one short line max." Collapse to one line, e.g.:

// Build inclusion from stored values (not name keys) so valid() agrees with scopes and is*() — ##3014.

The same rule applies to the multi-line inline comment blocks in structEnumValidationSpec.cfc (lines 17–21, 42–45, 55–60, 69–71). They explain what each test case does rather than why a non-obvious constraint exists, so they can simply be removed.

---

Correctness (no blocking findings)

The core logic is correct. ListToArray(local.enumDef.names) iterates the names returned by StructKeyList(), and local.enumDef.values[local.enumName] resolves the stored value via a case-insensitive struct lookup — so the inclusion list is built correctly regardless of whether Adobe CF uppercases struct keys vs. Lucee preserving case. ListAppend converts integer stored values (e.g. 0) to "0" as expected, and validatesInclusionOf does a loose comparison, so numeric stored values round-trip cleanly. The list form is unaffected because names == values there.

Cross-engine (no blocking findings)

The new for (local.enumName in ListToArray(...)) loop is standard CFML that runs identically on Lucee 5/6/7, Adobe CF 2018–2025, and BoxLang. No member-function collision, no closure scope concern, no bracket-notation call. The struct-literal argument in the spec (values={low: 0, medium: 1, high: 2}) is supported on all targeted engines. The spec follows the established g = application.wo pattern (assignment to variables scope, not local) so closure access to g inside it() and beforeEach()/afterEach() bodies is correct.

Commits

fix(model): struct-form enum() validates the stored value, matching scopes and is*() checkers — 95 chars, valid type, valid scope, not ALL-CAPS. DCO sign-off box ticked. ✓

Docs

Changelog fragment changelog.d/struct-enum-validation-mismatch.fixed.md is present and correctly uses the .fixed.md extension. CLAUDE.md and AI reference doc updates are deferred to the docs bot. ✓

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-reasoned. Both findings check out against the actual diff: the missing name-rejection assertion is a real test gap, and the multi-line comment blocks genuinely violate CLAUDE.md. Converging on changes-needed.

Sycophancy

None detected. A required concrete changes before marking ready for review and gave no unearned approval.

False positives

None detected.

• A's cross-engine analysis was verified: local.enumDef.names = StructKeyList(arguments.values) at line 907 of properties.cfc, and the lookup local.enumDef.values[local.enumName] is case-insensitive, so the fix is correct on all engines regardless of Adobe CF's key uppercasing.
• The correctness claim about ListAppend converting integers to strings and validatesInclusionOf doing loose comparison is consistent with CFML semantics.
• A's citation of CLAUDE.md §Tone for the multi-line comment rule is accurate. Both the production code block (lines 919–926) and the spec file blocks are violations.

Missed issues

None detected. I re-scanned the diff for:

• Cross-engine compat: no member-function collisions, no closure scope issues, no arguments scope passed to built-in tags.
• Test coverage beyond A's finding: the five existing tests cover the fix from several angles; the only gap is exactly the one A identified — no assertion that "low" (the name key) is now rejected.
• Commit conventions: title is 94 chars, valid type and scope, not ALL-CAPS, DCO ticked. ✓
• Changelog fragment: present and correctly named. ✓
• Security: no user-controlled input enters the inclusion list; enum values are developer-defined constants. No concern.

Verdict alignment

A's COMMENTED verdict with an explicit "both should be addressed before marking ready for review" is consistent with the two actionable findings.

Convergence

Aligned. A identified a real test gap (no assertion that the enum name string "low" is rejected after the fix) and a style violation (multi-line comment blocks in both production code and spec). I have no unaddressed findings of my own and no false-positive disputes. Joint recommendation: request changes — add one it() that assigns the name key and asserts valid() is false, and collapse the multi-line comment blocks to single lines (or remove the explanatory spec comments entirely per CLAUDE.md).

[wheels-bot]

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

• Missing assertion: struct-form enum NAME rejected. Added it("rejects the enum NAME string (struct form) — stored value is what matters") at vendor/wheels/tests/specs/model/structEnumValidationSpec.cfc:29 — assigns instance.lastName = "low" and asserts valid() is false. This closes the regression-detection gap A flagged and B independently confirmed: prior to the fix validatesInclusionOf accepted "low"; after the fix it must reject it, and that "rejects" half had no direct assertion.
• Multi-line comment block in production code. Collapsed the 8-line block at vendor/wheels/model/properties.cfc:919-926 to a single line: // Build inclusion from stored values (not name keys) so valid() agrees with scopes and is*() — #3014. per CLAUDE.md §Tone (one short line max).
• Multi-line inline comments in spec. Removed the explanatory comment blocks from the four it() bodies in structEnumValidationSpec.cfc (was at lines 17–21, 42–45, 55–60, 69–71) — the test descriptions and assertions are self-explanatory.

Test run note: local sandbox has no wheels CLI installed and tools/test-local.sh shells sed -i '' (macOS form), so the bash tools/test-local.sh model re-run aborted at the placeholder-resolve step without entering the suite. CI's cross-engine compat-matrix will exercise the suite on the new SHA — the change is pure CFML, uses the same g.model("author") / expect() patterns as the surrounding tests, and adds no new framework surface.

The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on approve or the outer-loop cap (5 rounds) is reached.

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This is the round-2 push addressing the two consensus findings from the first-round review. Both are fully resolved: the missing "rejects enum NAME string" assertion is now present as test 2 (instance.lastName = "low" → toBeFalse()), and all multi-line inline comment blocks have been removed from the spec and the implementation comment in properties.cfc is correctly single-line. The fix logic and all six test cases are correct. The only remaining nit is the fixup commit's subject line, which references the review process rather than the code change — a CLAUDE.md style note, and moot for a squash-merge workflow where the PR title is the landing commit. Verdict: comment.

---

Conventions

Commit 2 subject references the review process, not the code change

c816da647 fix(model): address Reviewer A/B consensus findings (round 1)

CLAUDE.md § "Commit Message Conventions": "Do not reference the current task, fix, or callers … since those belong in the PR description and rot as the codebase evolves." A subject like add name-rejection test and collapse multi-line inline comments would describe what actually changed in that commit. This is moot for the landing squash-merge commit (the PR title fix(model): struct-form enum() validates the stored value, matching scopes and is*() checkers is correct and clean), but worth noting for branch-history hygiene.

---

Correctness (no findings)

The core change in vendor/wheels/model/properties.cfc lines 920–922 correctly iterates local.enumDef.names (set from StructKeyList(arguments.values) at line 907) and resolves each stored value via a case-insensitive struct lookup local.enumDef.values[local.enumName]. ListAppend coerces integer stored values (e.g. 0) to "0", which round-trips cleanly through validatesInclusionOf's loose CFML string comparison. The list-form code path is unaffected (names == values there).

Cross-engine (no findings)

for (local.enumName in ListToArray(local.enumDef.names)) is standard CFML with no member-function collision, no closure-scope concern, and no bracket-notation parser crash. The struct lookup local.enumDef.values[local.enumName] is case-insensitive on all targeted engines, so the fix is correct regardless of whether Adobe CF uppercases struct literal keys or Lucee/BoxLang preserves them.

Tests (no findings)

All six test cases are present and correctly scoped:

1. Stored value 0 accepted by valid() with errorsOn("lastName") confirming zero errors
2. Enum name string "low" rejected — the previously missing assertion, now present
3. Unrelated string "bogus" rejected
4. The passing value is exactly the value isLow() returns true for
5. The passing value matches scopes.low.whereParams[1].value (scope-alignment proof)
6. List-form regression guard (names == values path still correct)

g = application.wo is a bare (unqualified) assignment inside function run(), so it lands in the CFC's variables scope — not local — and is correctly visible inside all it() / beforeEach() closures. The $clearModelInitializationCache() guard in beforeEach/afterEach prevents enum registrations from leaking across tests.

Docs (no findings)

changelog.d/struct-enum-validation-mismatch.fixed.md present with the correct .fixed.md extension. No direct CHANGELOG.md edit was made.

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is accurate on substance: the fix is correct, the tests exercise what they claim, and the cross-engine reasoning holds. One verdict inconsistency worth noting — A called its only finding explicitly moot, then chose comment instead of approve. The code is clean; the conservative verdict is harmless but slightly off. One cosmetic miss in the implementation comment. Converging to approve.

Sycophancy

None detected. Every positive claim A made is backed by specific code lines or reasoning (the ListAppend coercion note, the case-insensitive struct lookup, the variables-scope analysis).

False positives

None detected.

• A's claim that g = application.wo lands in variables scope (not local) and is therefore visible inside closures is correct. Bare assignments in a CFC function body go to variables; Anti-Pattern Fixed bug in $findRoute() that causes blow up on unmatched named route #10 in CLAUDE.md addresses var/local specifically — this pattern avoids that pitfall correctly.
• A's claim that local.enumDef.values[local.enumName] is case-insensitive on all targeted engines is correct.
• A's ListAppend coercion note (integer 0 → string "0", round-trips through validatesInclusionOf) is correct.

Missed issues

##3014 in a // code comment (cosmetic, not a bug)

vendor/wheels/model/properties.cfc line 919:

// Build inclusion from stored values (not name keys) so valid() agrees with scopes and is*() — ##3014.

Inside a // comment, CFML does not evaluate # expressions — no escaping is needed or meaningful. ##3014 will appear literally as ##3014 in the source. It should be #3014. Not a runtime bug; purely cosmetic. Below the bar for blocking merge.

Verdict alignment

A found zero actionable issues and explicitly called its one nit "moot for a squash-merge workflow," yet chose comment instead of approve. approve would be the consistent choice when the only finding is self-described as moot and no request-changes findings remain.

Convergence

Aligned. A's analysis of the fix, cross-engine safety, and test coverage is accurate. The comment vs approve delta is cosmetic given A's own "moot" annotation, and the missed ##3014 cosmetic note is not merge-blocking. Joint recommendation: approve and merge.