Skip to content

let a host configure diagnostics - #51

Open
hcollinsblox wants to merge 7 commits into
mainfrom
configurable-diagnostics
Open

hcollinsblox wants to merge 7 commits into
mainfrom
configurable-diagnostics

Conversation

@hcollinsblox

Copy link
Copy Markdown
Contributor

configure turns on reporting that costs nothing when it is off, since what counts as a problem depends on the runtime driving these signals:

warnScopelessReads reports a read that passed no scope at all, which silently never re-runs. An explicit false is left alone, being a deliberate untracked read. Reports are deduplicated by call site, because the ones worth finding are inside an effect that runs constantly, and debug.traceback is too expensive to pay per read.

showInternals attaches a debugState table to each signal and computed for an inspector to read. It is off by default because it is an allocation per source.

report directs both at the host's own log rather than warn.

Signals and computeds take a debug name to make any of that legible. Naming a computed is more common than giving it a comparison, so a string in the second position is taken as the name.

`configure` turns on reporting that costs nothing when it is off, since what
counts as a problem depends on the runtime driving these signals:

`warnScopelessReads` reports a read that passed no scope at all, which
silently never re-runs. An explicit `false` is left alone, being a deliberate
untracked read. Reports are deduplicated by call site, because the ones worth
finding are inside an effect that runs constantly, and `debug.traceback` is
too expensive to pay per read.

`showInternals` attaches a `debugState` table to each signal and computed for
an inspector to read. It is off by default because it is an allocation per
source.

`report` directs both at the host's own log rather than `warn`.

Signals and computeds take a debug name to make any of that legible. Naming a
computed is more common than giving it a comparison, so a string in the second
position is taken as the name.

Co-authored-by: Cursor <cursoragent@cursor.com>

@MisterUncloaked MisterUncloaked left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drafted by my agent and reviewed by me.

One must-fix and a few should-fix items inline.

Comment thread modules/signals/src/Signals.lua Resolved
Comment thread modules/signals/src/Signals.lua Resolved
Comment thread modules/signals/src/Signals.lua Resolved
Comment thread modules/signals/src/Signals.lua Resolved
Comment thread modules/signals/src/Signals.lua Resolved
@hcollinsblox
hcollinsblox requested review from MisterUncloaked and removed request for JasonRodrigues15 October 21, 2026 16:04

@MisterUncloaked MisterUncloaked left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drafted by my agent and reviewed by me.

LGTM, analyze is clean locally with the any return.

This branch is waiting to be deployed

1 waiting deployment
integration — b8ecafc3 Waiting Oct 27, 2026 by hcollinsblox via ci #114
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