Fix visualizer settings being clobbered by a minimal app override (breaks all rule runs) - #24
Merged
Merged
Conversation
ColdBox merges an app's moduleSettings over a module's defaults with a
shallow append, so the documented minimal config
`visualizer = { enabled = true }` replaced the whole default visualizer
struct and dropped metricsStore/datasourceName. RuleEventBus.onDIComplete
then threw on the missing key (its catch block re-read the same key, so
the error escaped), the bus was never built, and every RuleBook run failed
with a null publish() call.
- ModuleConfig: single visualizerDefaults() source; onLoad() deep-merges
the defaults under whatever the app supplied.
- RuleEventBus, Visualizer handler, SQLiteMetricsStore: tolerate missing
visualizer keys (defence in depth); resolveMetricsStore() no longer reads
settings inside its own catch block.
- Add VisualizerSettingsSpec.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
ModuleConfig is now the single place that sets up and checks the visualizer settings: onLoad() deep-merges the app's values over visualizerDefaults(), then throws RuleBox.InvalidSettingException for a value of the wrong shape. The handler, RuleEventBus and SQLiteMetricsStore go back to reading the settings directly, with no scattered fallbacks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
lmajano
pushed a commit
that referenced
this pull request
Oct 3, 2026
The handler reads visualizer.maxStreams directly; ModuleConfig fills the default of 25 and rejects anything that is not a whole number of 1 or more at load. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
lmajano
pushed a commit
that referenced
this pull request
Oct 3, 2026
retentionDays, maxStoredEvents, circuitBreakerThreshold and circuitBreakerCooldownSeconds get their defaults and whole-number validation in ModuleConfig. SQLiteMetricsStore reads them directly; numericSetting() is gone. Specs build store settings from the module's completed settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
lmajano
commented
Oct 3, 2026
| * The default `visualizer` settings: the single source of truth for both configure() and the | ||
| * onLoad() back-fill of keys an app's (shallow-merged) override left out. | ||
| */ | ||
| private struct function visualizerDefaults(){ |
Contributor
Author
There was a problem hiding this comment.
Why is int this static
Contributor
Author
There was a problem hiding this comment.
Good call, it has no instance state. Made it private static struct function visualizerDefaults() in 58c7583 (same change on #35 and #36 so they still merge cleanly). Each call still returns a fresh struct, so the deep-merge in onLoad() never mutates shared defaults. Full suite: 132 passed, 0 failed.
Generated by Claude Code
lmajano
commented
Oct 3, 2026
The defaults do not depend on instance state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
lmajano
commented
Oct 3, 2026
| * @return A new struct of defaults on every call, so callers can merge into it safely | ||
| */ | ||
| private static struct function visualizerDefaults(){ | ||
| return { |
Contributor
Author
There was a problem hiding this comment.
Why. A module is loaded once this should be a static variable
Contributor
Author
There was a problem hiding this comment.
Agreed. In db66c6d the defaults are a static VISUALIZER_DEFAULTS struct in a static { } block. configure() copies it once, and onLoad() fills the keys an app left out with append( static.VISUALIZER_DEFAULTS, false ), so the static is never written to. validateVisualizerSettings() is now void. Full suite: 132 passed, 0 failed. Merged into #35 and #36 as well.
Generated by Claude Code
A module is loaded once, so the defaults are a static struct instead of a function. configure() copies it, and onLoad() fills the keys an app left out with append( static.VISUALIZER_DEFAULTS, false ), so the static is never written to. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Failing scenario.
docs/guides/visualizer.mdtells people to enable the visualizer withvariables.moduleSettings = { rulebox = { visualizer = { enabled = true } } }. ColdBox merges an app's module settings over a module's defaults with a shallow append, so that replaces the whole defaultvisualizerstruct andmetricsStore/datasourceNamevanish.RuleEventBus.onDIComplete()then throwsThe key [metricsStore] was not found in the struct, the bus is never built, and every rule run fails withCannot invoke method [publish()] on a null objectbecauseRuleBookinjects the bus. The test harness hid this becausetest-harness/config/Coldbox.bxsetsmetricsStoreexplicitly. Apps leaving the visualizer off were unaffected.Fix.
ModuleConfig.bxis the single place that sets up and validates the visualizer settings:visualizerDefaults()holds the defaults;configure()uses it.onLoad()deep-merges the app'svisualizervalues over those defaults (app values win), then validates them. A value of the wrong shape throwsRuleBox.InvalidSettingExceptionnaming the key, so a bad config fails on app start:visualizermust be a structenabledmust be a booleanmetricsStoreanddatasourceNamemust be non-blankRuleEventBusandSQLiteMetricsStoreare unchanged: they read the settings directly and trust them. No fallbacks are scattered across readers.Verification. TestBox on BoxLang 1.18.0 (miniserver, private TestBox copy):
VisualizerSettingsSpec6/6 pass, full suite 132 passed, 0 failed, 6 skipped (pre-existing skips).New spec
test-harness/tests/specs/VisualizerSettingsSpec.bx:onLoad()back-fills a minimal override and keeps supplied values; it throwsRuleBox.InvalidSettingExceptionfor a badenabled, a blankmetricsStore, a complexdatasourceNameand a non-structvisualizer;RuleEventBusresolvesInMemoryMetricsStore@ruleboxand survives an unresolvable store; aRuleBookrun still succeeds and publishes an event under the minimal settings.Issues
Release blocker found while preparing 2.0.0 (no separate issue filed).
Type of change
Checklist
Follow-ups
ModuleConfigtoo.🤖 Generated with Claude Code
https://claude.ai/code/session_01Yc5MDcm3tyR2yqPWs9RpgX