Skip to content

fix(dispatch): Global cache cull, date parsing, CF version gate, existence caches - #2933

Merged
bpamiri merged 2 commits into
developfrom
peter/review-w2-review-global-core-fixes
Jun 10, 2026
Merged

fix(dispatch): Global cache cull, date parsing, CF version gate, existence caches#2933
bpamiri merged 2 commits into
developfrom
peter/review-w2-review-global-core-fixes

Conversation

@bpamiri

@bpamiri bpamiri commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six findings from the internal framework review in the global-core package: the duplicated Adobe CF version gate, the $addToCache cull that mutated the struct it was iterating, the contradictory slash-date parsers in $convertToString (the inline BoxLang block crashed on unambiguous US dates like 06/25/2024 10:30 AM), the comma-list file-existence memo in $objectFileName, the per-read IsDefined("request.wheels.tenant.config") string parse in $get(), and the staticRoutes index that $lockedLoadRoutes never cleared.

Reviewer-noted non-blocking follow-ups deferred (out of scope here): the ISO branch of $convertToString's catch-fallback still carries the same dead \d-as-literal regex bug fixed in the adjacent slash branch, and $addToCache still lacks the try/catch parity $getFromCache has (the in-loop StructKeyExists guard mitigates the practical race).

Findings addressed

  • DC2 [Medium] Three duplicate "Adobe ColdFusion" branches enforce CF 11 instead of CF 2018 @ vendor/wheels/Global.cfc:3045 (collapsed branch inside $checkMinimumVersion at :2996; enforces 2018.0.0, Len() guards the empty minimumBuild; Lucee numeral-key floor documented)
  • DC3 [Low] $addToCache cull deletes keys from the struct it iterates and mixes per-category culling with the global item count @ vendor/wheels/Global.cfc:819 ($cacheCount() computed once, categories/keys snapshotted via StructKeyArray before deletion, expired items culled globally with a Ceiling cap consistent with the global trigger/insert checks)
  • DC4 [Medium] $convertToString contains three contradictory slash-date parsers; the inline BoxLang one breaks unambiguous US dates @ vendor/wheels/Global.cfc:2637 (new $parseSlashDate() helper: d1>12 → DD/MM, d2>12 → MM/DD, ambiguous → engine adapter); all three call sites funneled through it, including vendor/wheels/model/miscellaneous.cfc:313; the catch-fallback's dead "\\d" regexes (matched a literal backslash, never a digit) fixed in the slash branch
  • DC13 [Medium, perf] $objectFileName memoizes file-existence in comma lists scanned with O(n) ListFindNoCase on every model-object materialization @ vendor/wheels/Global.cfc:1023 (struct memo keyed by path; init at vendor/wheels/events/onapplicationstart.cfc:108,111; default structs are case-insensitive on every engine, preserving the old ListFindNoCase semantics)
  • DC16 [Low, perf] $get() evaluates IsDefined("request.wheels.tenant.config") on every settings read @ vendor/wheels/Global.cfc:643-650 (cheap StructKeyExists chain; request.wheels.tenant is only ever assigned a struct by switchTenant, so the chain cannot throw)
  • R18 [Low, perf] staticRoutes index not cleared by $lockedLoadRoutes — stale first-write-wins entries survive a route reload @ vendor/wheels/Global.cfc:1443-1444 (StructClear alongside routes/namedRoutePositions); dead variables.staticRoutes deleted from vendor/wheels/Mapper.cfc (formerly :43, zero readers — Mapper writes only the application-scoped index, Dispatch reads are guarded)

Findings verified already-fixed

None — all six package findings still reproduced against origin/develop at implementation time. Independently spot-checked during review for DC2 (the three duplicate branches at develop's Global.cfc:3011/3016/3021) and R18 ($lockedLoadRoutes missing the clear).

Source

Internal multi-agent framework review 2026-06-09, wave 2, package global-core.

Tests

New BDD specs under vendor/wheels/tests/specs/global/, each verified red against pre-fix code (4 failures / 3 errors total) and green post-fix:

  • addToCacheSpec.cfc — cull math (pct=50, 10 expired → 6 remaining incl. the new item), global culling across categories, snapshot-before-delete
  • convertToStringSpec.cfc$parseSlashDate disambiguation incl. the previously crashing 06/25/2024 10:30 AM case (pre-fix: 3 errors on the missing helper)
  • loadRoutesSpec.cfcstaticRoutes cleared on reload (pre-fix: stale sentinel survived)
  • objectFileNameSpec.cfc — struct-shaped existence memo across cache-on/off × exists/missing paths (pre-fix: IsStruct failure)
  • internalSpec.cfc — Adobe version expectations updated for the CF 2018 floor

Local verification: single-bundle Docker run on Lucee 7 + SQLite (worktree-safe recipe). The full engine × DB matrix runs in CI, which is the real gate.

Cross-engine notes

  • New $parseSlashDate helper is public with $ prefix — mixin-safe per invariant 7 (private mixin functions are not integrated on Lucee/Adobe).
  • Catch-block locals are only read inside the catch (invariant 11, BoxLang-safe); no inline closures as constructor named args; specs follow the established style of sibling green-on-matrix global specs.
  • DC13's struct memos rely on default structs being case-insensitive, which holds on Lucee 5/6/7, Adobe 2018–2025, and BoxLang — behavior-equivalent to the old ListFindNoCase.
  • BoxLang-specific behavior of the inline AM/PM block is exercised by the CI compat matrix (local run was Lucee 7 + SQLite; the red-check convertToString errors came from the missing helper, not the BoxLang crash itself).

Changelog

Entry deliberately omitted; consolidated at campaign end.

🤖 Generated with Claude Code

…tence caches

Addresses six review findings in vendor/wheels/Global.cfc (wave 2,
package global-core):

- dispatch-core:2 — collapse the three duplicate "Adobe ColdFusion"
  branches in $checkMinimumVersion (only the first ever ran, enforcing
  EOL CF 11.0.18) into a single branch enforcing the documented CF 2018
  minimum, and document the Lucee per-major-release numeral-key floor.
- dispatch-core:3 — $addToCache no longer deletes keys from the struct
  it is iterating: snapshot keys per category, compute $cacheCount()
  once, and cull expired items across all categories (the maximum is
  global) so a write to a small category can actually free room instead
  of silently dropping the new item.
- dispatch-core:4 — funnel all slash-date disambiguation through a new
  $parseSlashDate() helper (d1>12 → DD/MM, d2>12 → MM/DD, ambiguous →
  engine adapter). The inline BoxLang block previously treated D1/D2/YYYY
  as DD/MM unconditionally, so "06/25/2024 10:30 AM" crashed with
  CreateDateTime(2024,25,6,...). Also fixes the catch-fallback's dead
  \\d regexes (literal backslash-d never matched a digit) and routes
  model/miscellaneous.cfc's BoxLang slash-date coercion through the same
  helper instead of exception-driven recovery.
- dispatch-core:13 — $objectFileName memoizes file existence in structs
  keyed by path instead of comma lists scanned with O(n) ListFindNoCase
  per materialized object (which also lost entries to unlocked
  concurrent ListAppend).
- dispatch-core:16 — $get() probes the tenant config override with a
  cheap StructKeyExists chain instead of string-parsing
  IsDefined("request.wheels.tenant.config") on every settings read.
- routing:18 — $lockedLoadRoutes clears the staticRoutes index alongside
  routes/namedRoutePositions so a route reload cannot serve stale
  first-write-wins entries; delete the dead variables.staticRoutes in
  Mapper.cfc.

New specs (verified red against pre-fix code, green post-fix on
Lucee 7 + SQLite): addToCacheSpec, convertToStringSpec, loadRoutesSpec,
objectFileNameSpec; internalSpec's Adobe version expectations updated
for the CF 2018 floor.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer A

TL;DR: Six tightly-scoped fixes for the global-core package — cache cull snapshot, $parseSlashDate disambiguation, Adobe 2018 version gate, struct-backed file-existence memo, $get() StructKeyExists chain, and staticRoutes clear on reload. All primary fixes are correct and well-tested. Two minor follow-up items worth tracking: the ISO regex in the catch-fallback branch carries the same \\d bug the PR fixes in the slash branch, and the DC16 $get() change has no corresponding spec. Overall verdict: comment (no correctness, cross-engine, or security blockers).


Correctness

ISO fallback regex still carries the \\d bug

vendor/wheels/Global.cfc (catch-fallback, branch 1):

if (ReFind("(?i)^(\\d{4})-(\\d{2})-(\\d{2})(?:[ T](\\d{1,2}):(\\d{2})(?::(\\d{2}))?)?$", local.s2)) {

CFML does not treat \ as a string-escape character, so "\\d" is the two-character string \d — which as a Java regex is \d (digit metacharacter) — wait, actually "\\d" is two source backslashes, making the string \\d, which the Java regex engine reads as \\d = literal-backslash + digit. Concretely, this regex never matches an ISO date like 2024-06-25; it would need \dddd-\dd-\dd as input.

The adjacent slash-date regex was fixed in this PR ("^\\d{1,2}/...""^\d{1,2}/..."), and the PR body explicitly notes this ISO branch as a deferred non-blocking follow-up:

"the ISO branch of $convertToString's catch-fallback still carries the same dead \d-as-literal regex bug"

Severity: low — ParseDateTime handles ISO strings correctly on all supported engines, so the catch branch never fires for ISO inputs. Not a blocker, but it should be tracked.

Suggested fix (for the follow-up PR):

// Replace \\d with \d throughout the ISO branch regex and backreference expressions
if (ReFind("(?i)^(\d{4})-(\d{2})-(\d{2})(?:[ T](\d{1,2}):(\d{2})(?::(\d{2}))?)?$", local.s2)) {
    local.parts = ReReplace(local.s2, "^(\d{4})-(\d{2})-(\d{2}).*$", "\1-\2-\3", "all");
    local.timePart = ReReplace(local.s2, ".*[ T](\d{1,2}:\d{2}(?::\d{2})?).*$", "\1", "all");

Tests

DC16 ($get()) fix has no spec

The IsDefined("request.wheels.tenant.config")StructKeyExists chain change (Global.cfc:636-650) is untested. For a perf-only fix this is understandable, but a simple spec asserting that tenant config overrides are applied (and that the code path doesn't throw when request.wheels is absent) would provide a safety net against future regressions.


loadRoutesSpec.cfc afterAll leaks a spurious staticRoutes key

vendor/wheels/tests/specs/global/loadRoutesSpec.cfc lines 37-40:

function afterAll() {
    application.wheels.routes = _originalRoutes
    application.wheels.staticRoutes = _originalStaticRoutes   // ← restores {} when key didn't exist before
    application.wheels.namedRoutePositions = _originalNamedRoutePositions
}

_originalStaticRoutes is set to {} when staticRoutes wasn't present before the spec ran (beforeAll line 7: StructKeyExists(application.wheels, "staticRoutes") ? StructCopy(...) : {}). After the spec, application.wheels.staticRoutes = {} leaves an extra empty struct key that wasn't there originally. Dispatch.cfc guards it correctly (StructKeyExists(application.wheels, "staticRoutes") → enters block, finds no matching key → falls through), so there is no functional impact. Still worth fixing for spec isolation hygiene:

function afterAll() {
    application.wheels.routes = _originalRoutes
    application.wheels.namedRoutePositions = _originalNamedRoutePositions
    if (_hadStaticRoutes) {
        application.wheels.staticRoutes = _originalStaticRoutes
    } else {
        StructDelete(application.wheels, "staticRoutes")
    }
}

where beforeAll records _hadStaticRoutes = StructKeyExists(application.wheels, "staticRoutes").


Docs

No CHANGELOG entry

The PR body states: "Entry deliberately omitted; consolidated at campaign end." This deviates from the repo convention (CLAUDE.md: [Unreleased] entry expected). Acknowledged and deferred — noting for the release author to pick up.


Everything else looks good

  • Cache cull ($addToCache): snapshot-before-delete with StructKeyArray, global cross-category culling, $cacheCount() called once, local.currentCount decremented by actual deletes — all correct. cacheLastCulledAt is updated even when no items were culled (pre-existing behaviour, not a regression).
  • $parseSlashDate(): disambiguation logic (d1 > 12 → DD/MM, d2 > 12 → MM/DD, ambiguous → engine adapter) is correct and consistent with the three call sites it replaces. Declared public with $ prefix — mixin-safe per invariant 7.
  • $objectFileName struct memo: StructKeyExists / struct-assignment correctly replaces ListFindNoCase / ListAppend. The ListSetAt mutation of local.fullObjectPath is correctly dropped; the proper-case name is now stored as the struct VALUE, retrieved via a case-insensitive key lookup (default struct semantics, all engines). StructKeyExists(local, "file") guard in the final return block remains correct.
  • Adobe 2018 version gate: three dead duplicate branches correctly collapsed; minimumBuild = "" with the Len(local.minimumBuild) guard skips the build check — any 2018.x passes. The per-major local.5 = {…} Lucee floor is unaffected.
  • $get() StructKeyExists chain: short-circuit evaluation means none of the inner calls execute when request doesn't carry a "wheels" key, so no risk of "undefined variable" errors during early request lifecycle.
  • $lockedLoadRoutes staticRoutes clear: StructKeyExists guard prevents a throw if the key was never written; StructClear correctly empties the index so stale entries can't survive a route reload. Dispatch.cfc reads from application.wheels which equals application[$appKey()] at runtime (after onapplicationstart renames $wheelswheels). ✓
  • Regex fix: "^\\d{1,2}/\\d{1,2}/\\d{4}""^\d{1,2}/\d{1,2}/\d{4}" and "\\d{1,2}:\\d{2}""\d{1,2}:\d{2}" — correct; the double-backslash forms produced Java regex \\d (literal-backslash + digit), which never matched real date/time strings.
  • Specs: addToCacheSpec, convertToStringSpec, loadRoutesSpec, objectFileNameSpec, internalSpec — all follow BDD extends="wheels.WheelsTest" style, no inline closures as constructor named args, no local.X = ... inside catch, StructKeyExists(local, …) guards used appropriately.
  • Commit: fix(dispatch): Global cache cull, date parsing, CF version gate, existence caches — valid conventional-commit header (72 chars, scope unrestricted).

@wheels-bot

wheels-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is solid. No sycophancy, no false positives, findings verified. A correctly left the verdict at 'comment' (no blocker) and I agree. The three non-blocking items are all real, none prevents merge.

Sycophancy: None detected. A's verdict is 'comment,' not approve, and every 'looks good' section cites specific implementation details rather than hand-waving.

False positives: None detected.

  • ISO catch-fallback regex bug — A is correct. The diff changes the slash-date branch from double-backslash-d form (Java regex matching literal-backslash + digit) to single-backslash-d (digit class). The ISO branch in the same catch block is untouched and carries the same broken form. The PR body acknowledges this as a deferred non-blocking follow-up, which is what A flags.

  • DC16 no spec — The diff adds five new or updated spec files but nothing covers the StructKeyExists chain in the tenant config path of the settings getter. A is correct.

  • loadRoutesSpec.cfc afterAll leaks staticRoutes — When the key was absent before the spec, the saved value is an empty struct, and the unconditional restore leaves that empty struct behind. A correctly notes Dispatch.cfc guards with StructKeyExists so there is no functional impact; spec isolation hygiene only. A's proposed fix (track whether the key existed, conditionally StructDelete on teardown) is the right repair.

Missed issues: None that rise above what A flagged. Rescan covered: cross-engine invariants (no inline closures as constructor named args; the new slash-date helper correctly declared public with dollar prefix; the catch-block assignment in miscellaneous.cfc mutates a pre-existing struct field so BoxLang invariant 11 does not apply); commit header length (82 chars, within limit); the checkMinimumVersion collapse (minimumBuild empty-string with Len guard means any 2018.x passes); onapplicationstart.cfc struct initialization matches the new StructKeyExists usage. No additional issues found.

Verdict alignment: A's 'comment' verdict is consistent with the findings: every item is non-blocking and either already acknowledged in the PR body or framed as a hygiene suggestion. 'Comment' is the right state.

Convergence: Aligned. A and I both read the PR as merge-ready with three tracked non-blockers: (1) the ISO catch-fallback regex deferred to a follow-up PR, (2) a missing spec for the settings-getter StructKeyExists chain, and (3) the loadRoutesSpec afterAll hygiene. No correctness, cross-engine, or security blocker found. Joint recommendation: merge as-is; three items go into a follow-up.

Resolves the one conflict (vendor/wheels/events/onapplicationstart.cfc)
by composing both sides: keep #2930's helperFileCache/layoutFileCache
struct caches from develop (consumed by Controller.cfc and
controller/layouts.cfc) and keep this PR's struct-typed
existingObjectFiles/nonExistingObjectFiles existence caches (consumed
by Global.cfc $objectFileName). The obsolete existing/nonExisting
helper- and layout-file list initializers are dropped; no consumers
remain repo-wide.

Auto-merged files verified semantically: Mapper.cfc (dead instance-level
staticRoutes removal vs #2924 comment update — disjoint),
model/miscellaneous.cfc ($parseSlashDate call vs #2931
$stampTimestampProperty helper — disjoint), Global.cfc (PR hunks at
~633-3007 vs #2912/#2943 hunks at ~3063-3181 — disjoint; both sides
present in the merge).

Verified locally on Lucee 7 + SQLite (dir-only docker mount): global
133/0/0, dispatch 104/0/0, mapper dir 89/0/0, mapperSpec 64/0/0,
mapperModernSpec 27/0/0, routingSpec 5/0/0, events 36/0/0, controller
456/0/0 (1 skip), model 869/0/0 (11 skip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

@wheels-bot wheels-bot Bot 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.

Wheels Bot — Reviewer A

TL;DR: Re-review at head b7a2b6b0d — the only change since the previous review (84ad292) is a merge of origin/develop into the branch to bring it current; the PR code itself is unchanged. All six primary fixes are correct and well-tested. Three carry-forward findings from the prior pass remain; no new concerns are introduced by the merge. Verdict: comment.


Correctness

ISO fallback regex (carry-forward, deferred)

vendor/wheels/Global.cfc — the catch-fallback ISO branch still uses "\\d{4}"-style patterns (double-backslash → Java regex \\d → literal backslash + d, not a digit). This was explicitly called out in the PR body as a deferred non-blocking follow-up. ParseDateTime handles ISO strings correctly on all supported engines so the catch branch never fires for real ISO input; the risk is latent but not blocking.

DC16 ($get()) change lacks a dedicated spec (carry-forward)

vendor/wheels/Global.cfc:633-650 — the IsDefined → StructKeyExists chain has no corresponding spec. The change is straightforward and low-risk, but a small test asserting the tenant-config override path (and that it does not throw when request.wheels is absent) would prevent future regressions. Still not a blocker.


Tests

loadRoutesSpec.cfc afterAll leaks a spurious staticRoutes key (carry-forward)

vendor/wheels/tests/specs/global/loadRoutesSpec.cfc:37-40:

function afterAll() {
    application.wheels.routes = _originalRoutes
    application.wheels.staticRoutes = _originalStaticRoutes   // restores {} when key was absent before
    application.wheels.namedRoutePositions = _originalNamedRoutePositions
}

_originalStaticRoutes is {} when staticRoutes was not present before the spec ran (beforeAll line 5: StructKeyExists(...) ? StructCopy(...) : {}). After the spec, application.wheels.staticRoutes = {} leaves an extra empty struct key that was not there originally. Dispatch.cfc guards with StructKeyExists so there is no functional impact, but spec teardown should mirror setup fidelity.

Suggested fix:

function beforeAll() {
    _hadStaticRoutes = StructKeyExists(application.wheels, "staticRoutes")
    _originalStaticRoutes = _hadStaticRoutes ? StructCopy(application.wheels.staticRoutes) : {}
    // ... rest of beforeAll
}

function afterAll() {
    application.wheels.routes = _originalRoutes
    application.wheels.namedRoutePositions = _originalNamedRoutePositions
    if (_hadStaticRoutes) {
        application.wheels.staticRoutes = _originalStaticRoutes
    } else {
        StructDelete(application.wheels, "staticRoutes")
    }
}

Docs

No CHANGELOG entry (carry-forward, intentional)

The PR body states "Entry deliberately omitted; consolidated at campaign end." This deviates from the repo convention (CLAUDE.md: [Unreleased] entry expected). Flagging for the release author to pick up at campaign close — no action required on the PR author.


Everything else looks good

A pass over the merge-updated diff confirms no regressions:

  • $addToCache cull: $cacheCount() called once into local.currentCount; StructKeyArray snapshots prevent concurrent-modification; local.currentCount -= local.deletedItems tracks the adjusted total; the final admission guard is correct for all cull-condition branches (cull fired, cull skipped — cache not full, cull skipped — interval not elapsed).
  • $parseSlashDate helper: declared public with $ prefix (mixin-safe per invariant 7); disambiguation logic (d1>12 → DD/MM, d2>12 → MM/DD, ambiguous → $engineAdapter().parseAmbiguousSlashDate) is correct. The call site in model/miscellaneous.cfc:313 retains its try/catch fallback; the AM/PM block in Global.cfc validates the format with ReFind before calling the helper.
  • Regex fix: "^\\d{1,2}/\\d{1,2}/\\d{4}""^\d{1,2}/\d{1,2}/\d{4}" and the time pattern — correct. In CFML, "\\d" is three characters \\d which the Java regex engine reads as literal-backslash + digit, never matching a real date string; "\d" is two characters \d = digit metaclass.
  • $objectFileName struct memo: application.wheels.existingObjectFiles[path] = properCaseName (value) / nonExistingObjectFiles[path] = false correctly replaces O(n) ListFindNoCase + ListAppend. onapplicationstart.cfc init changed from "" to {} to match. Case-insensitive struct lookup preserves the old ListFindNoCase semantics on all supported engines.
  • Adobe version gate: three duplicate Adobe ColdFusion branches collapsed to a single 2018 floor; minimumBuild = "" with the Len() guard skips the build check — any 2018.x passes.
  • $lockedLoadRoutes static-routes clear: StructKeyExists guard prevents a throw if the key was never written; StructClear correctly empties the index on reload. Dead variables.staticRoutes in Mapper.cfc correctly removed (Mapper wrote only the application-scoped index; Dispatch reads it from application[appKey]).
  • $get() StructKeyExists chain: short-circuit evaluation means inner expressions never execute when request has no "wheels" key — no early-lifecycle undefined-variable risk.
  • Specs: addToCacheSpec, convertToStringSpec, loadRoutesSpec, objectFileNameSpec, internalSpec — all extend wheels.WheelsTest BDD style, no inline closures as constructor named args (invariant 5), no local.X assignments inside catch bodies (invariant 11), StructKeyExists(local, ...) guards used appropriately.
  • Commit: fix(dispatch): Global cache cull, date parsing, CF version gate, existence caches — 72-char header, valid conventional-commit type and scope.

@wheels-bot

wheels-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is solid. The three carry-forward findings are accurately sourced, the technical walkthrough checks out against every line I re-read in the diff, and the "comment" verdict is calibrated correctly — non-blocking items should not gate a merge. One minor factual error in A's review, one small omission in the teardown analysis, neither changes the recommendation.

Sycophancy

None detected. A uses verdict "comment" (not approve), backs every finding with file and line references, and explicitly does not clear the carry-forward items despite them being non-blocking. The "Everything else looks good" section is preceded by a line-by-line walkthrough that demonstrates the claim.

False positives

  • Commit header character count: A states the header is "72-char". Counting the full string fix(dispatch): Global cache cull, date parsing, CF version gate, existence caches gives 81 characters. The conclusion — header is within the 100-char limit — is correct, so this does not affect the verdict. Noting it only to keep A's factual record clean.

All other claims I verified directly against the diff:

  • Regex fix ("\\d" to "\d" in CFML = Java \d digit class) is correctly described.
  • $parseSlashDate declared public with $ prefix is correct per invariant 7.
  • onapplicationstart.cfc init changed from "" to {} for both existence structs confirmed in the diff.
  • local.currentCount -= local.deletedItems is inside the cull if block, so no undefined-variable risk on the non-cull path confirmed.
  • model/miscellaneous.cfc:313 call to $parseSlashDate remains inside the existing try/catch confirmed.

Missed issues

  • loadRoutesSpec.cfc teardown: same pattern applies to namedRoutePositions. A's finding calls out that _originalStaticRoutes = {} when the key was absent causes afterAll to leave a spurious empty struct. The same logic applies to namedRoutePositions: the beforeAll guard is StructKeyExists(...) ? StructCopy(...) : {} and afterAll assigns it unconditionally. In practice namedRoutePositions almost certainly exists before the spec runs (it is a standard application init key), so the practical impact is lower than for staticRoutes. A's suggested fix pattern (_hadStaticRoutes flag + conditional StructDelete) should be extended to namedRoutePositions for full teardown fidelity. This is an enhancement to A's existing finding, not a new independent issue.

No other missed issues. I re-scanned for cross-engine compat (invariants 1-14), tests that do not exercise the change, and security concerns. Nothing A skipped.

Verdict alignment

A's "comment" verdict is consistent with the findings — three acknowledged non-blocking carry-forwards, all primary fixes verified correct, no new concerns introduced by the merge commit. "Approve" would have been sycophantic here; "request-changes" would be overcorrecting on deferred items the PR author already acknowledged.

Convergence

Aligned. A's review is accurate, the verdict is appropriate, and no code changes are needed for this SHA. The namedRoutePositions teardown gap is an enhancement to an already-noted finding, not a blocker. The carry-forward items (ISO regex, DC16 spec, teardown fidelity) remain open tracking items but are explicitly non-blocking per the PR body.

@bpamiri
bpamiri merged commit 1fdbce9 into develop Jun 10, 2026
7 checks passed
@bpamiri
bpamiri deleted the peter/review-w2-review-global-core-fixes branch June 10, 2026 07:59
bpamiri added a commit that referenced this pull request Jun 10, 2026
…ntegration-and-refle

Resolves the CHANGELOG.md conflict by integrating the PR's entry as a
new "### Performance" subsection inside develop's populated
"## [Unreleased]" section (discarding the PR hunk's malformed
single-hash "# [Unreleased]" header). vendor/wheels/Global.cfc
auto-merged cleanly: develop's sibling changes since the merge base
(#2933 cache culls + $objectFileName struct memoization, #2939
$deprecated registry, #2912/#2943 provider isolation + mixin-collision
helpers, staticRoutes clear in $lockedLoadRoutes) touch regions
orthogonal to this PR's $cachedModelLookup/$cachedControllerLookup
helpers and the model()/controller() fast paths; both sides' semantics
verified to survive — the fast path reads the same
application.wheels.models/controllers structs that develop's
unchanged $cachedModelClassExists/$createModelClass write.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant