fix(view): resolve guides sidebar dynamically so the in-app view loads after v4.0.0 GA - #2650
Conversation
…s after v4.0.0 GA The in-app Guides redirector at vendor/wheels/public/docs/guides.cfm and the AI summary endpoint at vendor/wheels/public/views/ai.cfm both hardcoded the sidebar path to v4-0-0-snapshot.json, which was removed when v4.0.0 went GA (the active dev snapshot is now v4-0-1-snapshot.json). The FileExists() guard then fell through silently, so monorepo contributors saw an empty Guides sidebar. Replace the hardcoded basename with a glob over web/sites/guides/src/ sidebars/*.json sorted descending lexicographically — a "-snapshot" suffix sorts after the matching GA, so the dev snapshot wins at the same version. The external redirect URL on guides.cfm is now derived from the same active slug so the two never drift apart again. The endpoints still return cleanly when the monorepo tree is absent (installed apps). Fixes #2647 Co-authored-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — No doc updatesReviewed this PR's diff and found no docs that need updating (internal bug fix that restores expected behavior — replaces hardcoded |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This PR fixes the in-app Guides sidebar and AI summary endpoint going silent for monorepo contributors after v4.0.0 GA renamed v4-0-0-snapshot.json. Replacing the hardcoded path with a DirectoryList glob + descending sort is the right approach, the fallback is correct, and DCO/commitlint/CHANGELOG are all in order. I am submitting a --comment review: one nit on a misleading sort-invariant claim in the comments, and one note on the test strategy. Neither is blocking.
Correctness
Sort-invariant comment states the opposite of what happens — vendor/wheels/public/docs/guides.cfm lines 14–15, vendor/wheels/public/views/ai.cfm lines 311–312, vendor/wheels/tests/specs/view/guidesSidebarDiscoverySpec.cfc lines 47–48.
All three locations repeat this claim:
// a "-snapshot" suffix sorts after the matching GA, which is what we want
// (the dev snapshot wins at the same version).It is backwards. In ASCII, . (46) > - (45), so v4-0-1.json sorts higher than v4-0-1-snapshot.json; the GA wins at the same version, not the snapshot:
$ printf '%s\n' "v4-0-1.json" "v4-0-1-snapshot.json" | sort -r
v4-0-1.json ← GA wins, not snapshot
v4-0-1-snapshot.json
In the current repo (v4-0-0.json + v4-0-1-snapshot.json) the code still picks correctly — v4-0-1-snapshot.json wins over v4-0-0.json because the version segment 4-0-1 > 4-0-0. There is no actual bug today. The problem surfaces only if both v4-0-1.json and v4-0-1-snapshot.json exist simultaneously (a transient state during the next GA cut), in which case the in-app redirect and AI summary would serve the GA slug instead of the active dev snapshot.
Suggested replacement for the comment (no code change needed; same in all three files):
// Sidebar basenames like "v4-0-1-snapshot.json" / "v4-0-0.json" sort
// sensibly in descending lexicographic order because the version segment
// (e.g. "4-0-1") dominates — the snapshot is always named at the NEXT
// minor version while GA files carry the released version. Note: at an
// identical version prefix, "-snapshot" sorts LOWER than ".json" (ASCII
// "." > "-"), so if "v4-0-1.json" and "v4-0-1-snapshot.json" ever
// coexist the GA wins; in practice only one exists at a time.Tests
Source-pinning tests are correct but brittle — vendor/wheels/tests/specs/view/guidesSidebarDiscoverySpec.cfc lines 19–32:
expect(source).notToInclude("v4-0-0-snapshot.json", ...);A future comment in guides.cfm or ai.cfm that mentions the retired filename for historical context (e.g., // Previously hardcoded to v4-0-0-snapshot.json) would cause a spurious failure. The PR acknowledges this is an established pattern in the repo (guidesRedirectHeadSpec.cfc, routesViewBrowserFixturesSpec.cfc) — that is fair, and no change is needed. Noting it here so the on-call maintainer knows to look at the spec when this fires after a future doc-comment edit rather than assuming a real regression.
Wheels Bot — Reviewer B (round 1)A's review is substantive and technically accurate on the key finding. The misleading sort-invariant comment is real and actionable. Minor over-claim on which files contain the wrong text, but not enough to reject the analysis. Converging with changes. SycophancyNone detected. A did not approve without verification and correctly withheld an approval-level verdict despite the fix being sound overall. False positivesOne minor overstatement. A says "all three locations repeat this claim" and includes // Discover the latest sidebar in the monorepo sidebars dir (snapshot
// or GA - whichever sorts highest). See vendor/wheels/public/docs/
// guides.cfm for the rationale; the same logic lives there."Whichever sorts highest" is neutral - it does not assert that the snapshot wins at the same version. The specific wrong claim ("a '-snapshot' suffix sorts after the matching GA") appears in Missed issuesNone detected. The sort-invariant and test-brittleness findings are the two items worth noting. Cross-engine compat (standard Verdict alignmentA's ConvergenceAligned with A's assessment. The comment fix A specifies is clear: update the misleading sort-invariant prose in |
Correct the misleading sort-invariant comment that asserted "-snapshot" sorts after the matching GA at the same version. ASCII "." (46) > "-" (45), so at an identical version prefix the GA wins, not the snapshot. The runtime behavior is still correct because the snapshot file is always named at the next minor version (so the version segment dominates) — only the explanatory prose was wrong. - vendor/wheels/public/docs/guides.cfm: replace the inverted invariant with A's suggested wording (version segment dominates; at identical prefix the GA wins; in practice only one of the pair coexists). - vendor/wheels/public/views/ai.cfm: inline the same precise rationale rather than only cross-referencing guides.cfm (B noted ai.cfm's prior text was neutral, but the explicit invariant is still an improvement and keeps the two files self-consistent). - vendor/wheels/tests/specs/view/guidesSidebarDiscoverySpec.cfc: update the test's commentary to match the corrected invariant. The brittleness note A raised about source-pinning tests is left unaddressed by design — A explicitly said no change is needed and the PR description already calls out the established repo pattern. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Skipped (not in consensus):
Test run: 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. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This is a follow-on review for the round-1 fix commit (362384e97) that addressed the sort-invariant comment inversion I raised in my previous review. The fix is correct and the PR is essentially clean. I am submitting a --comment review with one nit about stale prose in the PR description.
Correctness
The round-1 commit correctly updated the comments in all three files to reflect the true ASCII ordering (. (46) > - (45), so at an identical version prefix the GA wins, not the snapshot). Runtime behavior was always correct; only the explanatory prose was wrong. That is now fixed.
The discovery logic itself is sound:
DirectoryList(local.sidebarDir, false, "name", "*.json")returns a CFML array of basenames — consistent with the same call pattern used inSeeder.cfc,Controller.cfc,Model.cfc, andPackageLoader.cfc.ArraySort(local.candidates, "textnocase", "desc")sorts in-place; the code correctly readscandidates[1]afterward.Len(local.sidebarPath)gate beforeFileExists()prevents a null-path call in the no-monorepo-tree case.- All HTML output in
guides.cfmis properly encoded viaencodeForHTMLAttribute/encodeForHTML.
Docs
PR description still carries the old incorrect invariant.
The PR body (Summary section) says:
sorted descending lexicographically (a
-snapshotsuffix sorts after the matching GA at the same version, which is the behavior we want — the dev snapshot wins)
The round-1 commit corrected this claim in the code comments and test commentary, but the PR description was not updated. As written it now contradicts the corrected comments. Future readers who audit the PR history will see conflicting explanations.
No code change is needed. Updating the PR description before merging would avoid confusion:
sorted descending lexicographically — the version segment (e.g.
4-0-1) dominates, so the active snapshot (named at the next minor version) sorts above older GA files. At an identical version prefix-snapshotsorts LOWER than.json(ASCII.>-), so if both ever coexist the GA wins; in practice only one of the pair exists at a time.
The initial commit message body (dad10aa27) also repeats the wrong invariant, but that is a historical artifact — amending a pushed commit would require a force-push that outweighs the value of fixing explanatory prose.
Tests
Source-pinning checks are correct BDD style extending wheels.WheelsTest. The third test's early return for installed-app environments (no monorepo web/ tree) is the right skip pattern and leaves CI green. No concerns.
Commits
Both commits conform to commitlint.config.js: type fix, scope view, subjects within 100 chars, not ALL-CAPS. Both carry Signed-off-by: claude[bot] for DCO compliance.
Wheels Bot -- Reviewer B (round 1)A's review on the round-1 fix commit is technically accurate and appropriately scoped. The single finding -- stale PR description text -- is real, A's verdict (COMMENTED, no changes required) is proportionate, and the code review itself is substantive. Converging on approve. SycophancyNone detected. A did not rubber-stamp. Before declaring the code clean A verified False positivesNone detected. I re-read every claim A made against the diff:
Missed issuesNone detected. I scanned the full diff:
Verdict alignmentA's ConvergenceAligned. A's analysis is correct and the code is clean. The PR description nit is real and worth a human glance before merging, but it does not affect runtime behaviour, tests, or future maintainers reading the code (the code comments and spec commentary are already corrected). No further review iteration is needed from the bot. |
…-in-repo-setup-due-to
There was a problem hiding this comment.
Wheels Bot — Reviewer A
This is a third-pass review, triggered by the develop-sync merge adbf097 (merging upstream fixes from #2646 into the feature branch). The merge touches five files — .ai/wheels/cross-engine-compatibility.md, CHANGELOG.md, BoxLangAdapter.cfc, and two new dispatch test assets — none of which overlap with this PR's four files (guides.cfm, ai.cfm, guidesSidebarDiscoverySpec.cfc, CHANGELOG.md entry). The effective diff is identical to what round-2 reviewed at 362384e97. No new issues found. Submitting --comment.
Docs
PR description still carries the old sort-invariant — flagged in round-2, still present.
The Summary section reads:
sorted descending lexicographically (a
-snapshotsuffix sorts after the matching GA at the same version, which is the behavior we want — the dev snapshot wins)
The code comments were corrected by 362384e97, but the PR body was not updated. As written it contradicts the corrected code comments and test commentary. The replacement from round-2 still stands:
sorted descending lexicographically — the version segment (e.g.
4-0-1) dominates, so the active snapshot (named at the next minor version) sorts above older GA files. At an identical version prefix-snapshotsorts LOWER than.json(ASCII.>-), so if both ever coexist the GA wins; in practice only one of the pair exists at a time.
No code change needed; this is cosmetic prose in the PR history.
Correctness, Cross-engine, Tests, Security, Commits
No new findings. All are clean per round-2. The four files owned by this PR are unchanged from 362384e97.
Wheels Bot — Reviewer B (round 1)A's third-pass review is accurate and appropriately scoped. The develop-sync merge brings only BoxLang dispatch code (no overlap with this PR's four files), A verified that explicitly, and the sole outstanding item — the stale PR description prose — is unchanged from prior rounds. Converging on approve. SycophancyNone detected. A did not rubber-stamp the merge commit; it explicitly verified which files were introduced by the sync and confirmed zero overlap with the PR's owned files before re-applying the prior round's clean verdict. False positivesNone detected. I re-read the diff:
Missed issuesNone detected. The merged dispatch code is outside this PR's scope and carries its own tests. No cross-engine concerns arise from the sidebar files (standard Verdict alignmentA's ConvergenceAligned. The code is clean; this is a develop-sync merge that adds nothing to review. The PR description nit is real but affects only the PR history, not the code, tests, or future maintainers (the code comments and spec commentary were corrected in |
Reviewer A and B converged on two concerns at the prior head (b3b32ea) and one related workflow false-negative surfaced during this PR: - CHANGELOG.md: the latest develop merge re-introduced unresolved `<<<<<<<` / `=======` / `>>>>>>>` markers around the `### Fixed` section. Both entries (this branch's tab-init line and develop's guides-sidebar globbing line, #2650) are kept with markers removed. - coreTestRunnerTabsSpec.cfc:39: the second `it`'s position probe was searching for the bare substring `.tab()`, which also matches the CFML comment above the `<script>` block (the comment uses the `$()` jQuery alias). Tighten the search to `jQuery('.menu .item').tab()` — that exact string lives only in the executable script block, so both `it`s now lock to the same token. - .github/workflows/bot-tdd-gate.yml: the impl-detection regex excluded the entire `vendor/wheels/tests/` and `tests/` trees when computing "non-test changes". That treats the test-runner result- page UI (`vendor/wheels/tests/html.cfm`, rendered to users' browsers at `/wheels/core/tests`) as test code and flagged this PR as "specs but no implementation" even though the spec asserts against that very file. Narrow the exclusion to `tests/specs/` and `vendor/wheels/tests/specs/` so non-spec files under tests/* count as implementation when a bot fix touches them. Signed-off-by: Peter Amiri <peter@alurium.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y switchable (#2653) * fix(view): inline tab init on core test runner so full-suite tabs stay switchable The Failures / Errors / Passed tabs on `/wheels/core/tests` relied solely on `_footer.cfm`'s `$('.menu .item').tab();` for activation. On the full-suite path that footer JS does not always reach the browser, so every tab but the default-active one rendered as static markup. Bind the Semantic UI tabs inline, immediately after the menu, so switching works regardless of what happens further down the response. Fixes #2651 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(view): address Reviewer A/B consensus findings (round 1) - Resolve unresolved git conflict markers in CHANGELOG.md left in the develop merge; keep both `### Fixed` entries (this branch's tab-init fix and the develop `$blockInProduction` BoxLang dispatch fix). - Tighten the first assertion in `coreTestRunnerTabsSpec.cfc` to search for the exact JS call `jQuery('.menu .item').tab()`. The previous `.menu .item` / `.tab()` substring assertions were also satisfied by the CFML comment block above the script and so did not prove the executable `<script>` was present. The new search string appears only in the script block; the now-redundant second assertion in the same `it` is removed. The position test in the second `it` is unchanged. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(view): address Reviewer A/B round-2 findings Reviewer A and B converged on two concerns at the prior head (b3b32ea) and one related workflow false-negative surfaced during this PR: - CHANGELOG.md: the latest develop merge re-introduced unresolved `<<<<<<<` / `=======` / `>>>>>>>` markers around the `### Fixed` section. Both entries (this branch's tab-init line and develop's guides-sidebar globbing line, #2650) are kept with markers removed. - coreTestRunnerTabsSpec.cfc:39: the second `it`'s position probe was searching for the bare substring `.tab()`, which also matches the CFML comment above the `<script>` block (the comment uses the `$()` jQuery alias). Tighten the search to `jQuery('.menu .item').tab()` — that exact string lives only in the executable script block, so both `it`s now lock to the same token. - .github/workflows/bot-tdd-gate.yml: the impl-detection regex excluded the entire `vendor/wheels/tests/` and `tests/` trees when computing "non-test changes". That treats the test-runner result- page UI (`vendor/wheels/tests/html.cfm`, rendered to users' browsers at `/wheels/core/tests`) as test code and flagged this PR as "specs but no implementation" even though the spec asserts against that very file. Narrow the exclusion to `tests/specs/` and `vendor/wheels/tests/specs/` so non-spec files under tests/* count as implementation when a bot fix touches them. Signed-off-by: Peter Amiri <peter@alurium.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Peter Amiri <peter@alurium.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The in-app Guides view (
vendor/wheels/public/docs/guides.cfm) and the AI summary endpoint (vendor/wheels/public/views/ai.cfm) both hardcoded their sidebar source toweb/sites/guides/src/sidebars/v4-0-0-snapshot.json. That filename was removed when v4.0.0 went GA — the active dev snapshot is nowv4-0-1-snapshot.json— so theFileExists()guard fell through silently and monorepo contributors saw an empty Guides sidebar.This PR replaces the hardcoded basename with a glob over
web/sites/guides/src/sidebars/*.jsonsorted descending lexicographically (a-snapshotsuffix sorts after the matching GA at the same version, which is the behavior we want — the dev snapshot wins). The external redirect URL onguides.cfmis now derived from the same active slug so the two strings never drift apart again. Both endpoints still return cleanly when the monorepoweb/tree isn't on disk (installed-app environments).Related Issue
Fixes #2647
Type of Change
Feature Completeness Checklist
git commit -svendor/wheels/tests/specs/view/guidesSidebarDiscoverySpec.cfc(regression guard for Guides sidebar fails to load in repo setup due to missingv4-0-0-snapshot.jsonfile path #2647 —notToInclude("v4-0-0-snapshot.json")on both files, plus a dynamic-discovery existence check; this is the same source-pinning pattern already used byguidesRedirectHeadSpec.cfcandroutesViewBrowserFixturesSpec.cfc)bot-update-docs.ymlif the user-facing guide needs to changebot-update-docs.ymlbot-update-docs.yml[Unreleased] → Fixedbash tools/test-local.sh viewis not runnable in this bot environment (no Wheels CLI install, no Lucee Express); failing-then-passing TDD discipline was verified by static inspection of the spec against pre-fix and post-fix source. CI will run the full Lucee 7 / Adobe / SQLite / MySQL / Postgres matrix on this PR; please rely on those checks rather than the unchecked local box.Test Plan
vendor/wheels/tests/specs/view/guidesSidebarDiscoverySpec.cfcpasses on Lucee 7 + SQLite/wheels/docs/guidesin a fresh monorepo checkout — sidebar populates with sections fromweb/sites/guides/src/sidebars/v4-0-1-snapshot.json/wheels/ai?format=jsonreturns a non-emptyguidesarray under the documentation key when the monorepoweb/tree is presentguides, no exception) whenweb/sites/guides/src/sidebars/does not exist (simulated installed app)Notes for reviewer
local.activeSlug = "v4-0-0"inguides.cfmmeans installed apps redirect to the canonical GA docs URL rather than a stale snapshot slug.astro.config.mjsalready has redirects from/v4-0-0-snapshot/*→/v4-0-0/*for back-compat, so neither slug is broken externally; using the GA slug as the fallback is just the more honest default in non-snapshot deployments.cli/lucli/services/Doctor.cfc,cli/lucli/Module.cfc, scaffolded templates undercli/lucli/templates/app/, andcli/src/templates/ConfigRoutes.txt. Those are user-facing doc URLs in onboarding output / generated apps, not framework runtime paths, and Stale docs.cfwheels.org URL in scaffolded config/routes.cfm template comment #2635 already tracked the routes-template variant. I left them alone — the failure mode the original issue reports is the in-app sidebar, which is what this PR fixes.