Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Select an option

  • Save evnchn/9f5d500c09ed10c4ba5db9662e639c38 to your computer and use it in GitHub Desktop.

Select an option

Save evnchn/9f5d500c09ed10c4ba5db9662e639c38 to your computer and use it in GitHub Desktop.
Code review of evnchn-nicegui/nicegui-diagnostics PR #1 — empirical, multi-turn co-review (2026-05-22)
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>PR #1 review — nicegui-diagnostics — 2026-05-22</title>
<style>
:root {
--bg: #0d1117;
--panel: #161b22;
--panel-2: #1c2230;
--border: #30363d;
--text: #e6edf3;
--muted: #9ba8b8;
--accent: #58a6ff;
--warn: #f0883e;
--bad: #f85149;
--ok: #3fb950;
--soft: #d29922;
--mono: ui-monospace, SFMono-Regular, "JetBrains Mono", Menlo, Consolas, monospace;
}
html, body { background: var(--bg); color: var(--text); margin: 0; padding: 0; }
body { font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif; font-size: 14px; line-height: 1.55; }
.wrap { max-width: 1180px; margin: 0 auto; padding: 32px 24px 64px; }
header h1 { font-size: 24px; margin: 0 0 4px; }
header .meta { color: var(--muted); font-size: 13px; }
header .meta a { color: var(--accent); text-decoration: none; }
header .meta a:hover { text-decoration: underline; }
.grid { display: grid; grid-template-columns: repeat(auto-fit, minmax(220px, 1fr)); gap: 14px; margin: 24px 0; }
.stat { background: var(--panel); border: 1px solid var(--border); border-radius: 8px; padding: 14px 16px; }
.stat .label { font-size: 11px; text-transform: uppercase; letter-spacing: 0.06em; color: var(--muted); }
.stat .value { font-size: 22px; font-weight: 600; margin-top: 4px; font-family: var(--mono); }
.stat .sub { font-size: 12px; color: var(--muted); margin-top: 2px; }
section { margin: 28px 0; }
h2 { font-size: 17px; margin: 0 0 10px; padding-bottom: 6px; border-bottom: 1px solid var(--border); }
h2 .tag { font-size: 11px; padding: 2px 8px; border-radius: 999px; margin-left: 8px; vertical-align: middle; font-weight: 500; }
.tag-theme { background: rgba(88,166,255,0.12); color: var(--accent); border: 1px solid rgba(88,166,255,0.3); }
.tag-bad { background: rgba(248,81,73,0.12); color: var(--bad); border: 1px solid rgba(248,81,73,0.3); }
.tag-warn { background: rgba(240,136,62,0.12); color: var(--warn); border: 1px solid rgba(240,136,62,0.3); }
.tag-ok { background: rgba(63,185,80,0.12); color: var(--ok); border: 1px solid rgba(63,185,80,0.3); }
.tag-info { background: rgba(210,153,34,0.12); color: var(--soft); border: 1px solid rgba(210,153,34,0.3); }
table { width: 100%; border-collapse: collapse; background: var(--panel); border: 1px solid var(--border); border-radius: 8px; overflow: hidden; }
th, td { text-align: left; padding: 10px 12px; border-bottom: 1px solid var(--border); vertical-align: top; }
th { background: var(--panel-2); color: var(--muted); font-size: 12px; text-transform: uppercase; letter-spacing: 0.05em; font-weight: 600; }
tr:last-child td { border-bottom: none; }
td.sev { white-space: nowrap; width: 1%; }
td.where { font-family: var(--mono); font-size: 12px; color: var(--muted); white-space: nowrap; }
.pill { display: inline-block; font-size: 11px; padding: 2px 8px; border-radius: 4px; font-weight: 600; }
.p-high { background: rgba(248,81,73,0.15); color: var(--bad); }
.p-med { background: rgba(240,136,62,0.15); color: var(--warn); }
.p-low { background: rgba(210,153,34,0.15); color: var(--soft); }
.p-nit { background: rgba(155,168,184,0.15); color: var(--muted); }
.p-ok { background: rgba(63,185,80,0.15); color: var(--ok); }
code { font-family: var(--mono); background: rgba(110,118,129,0.18); padding: 1px 5px; border-radius: 3px; font-size: 0.92em; }
.quote { color: var(--muted); border-left: 3px solid var(--border); padding: 4px 10px; margin: 6px 0; font-style: italic; }
.cols { display: grid; grid-template-columns: 1fr 1fr; gap: 14px; }
.card { background: var(--panel); border: 1px solid var(--border); border-radius: 8px; padding: 14px 16px; }
.card h3 { font-size: 14px; margin: 0 0 8px; color: var(--text); }
ul.tight { margin: 6px 0 0 18px; padding: 0; }
ul.tight li { margin-bottom: 4px; }
.legend { display: flex; gap: 8px; flex-wrap: wrap; margin-top: 6px; }
footer { color: var(--muted); font-size: 12px; margin-top: 36px; border-top: 1px solid var(--border); padding-top: 12px; }
details { background: var(--panel); border: 1px solid var(--border); border-radius: 8px; padding: 10px 14px; margin-top: 10px; }
summary { cursor: pointer; color: var(--accent); font-weight: 600; }
.top3 li { margin-bottom: 8px; }
</style>
</head>
<body>
<div class="wrap">
<header>
<h1>PR #1 — <code>evnchn-nicegui/nicegui-diagnostics</code></h1>
<div class="meta">
Branch <code>setup/initial-environment</code> &middot;
Author <a href="https://github.com/Denubis">@Denubis</a> (Brian Ballsun-Stanton) + Claude Opus 4.7 &middot;
Reviewed 2026-05-22 by evnchn's Claude &middot;
<a href="https://github.com/evnchn-nicegui/nicegui-diagnostics/pull/1">PR link</a>
</div>
<div class="legend" style="margin-top:10px;">
<span class="pill p-high">High</span>
<span class="pill p-med">Med</span>
<span class="pill p-low">Low</span>
<span class="pill p-nit">Nit</span>
<span class="pill p-ok">Praise</span>
</div>
</header>
<div class="grid">
<div class="stat"><div class="label">Files changed</div><div class="value">8</div><div class="sub">3 of them docs</div></div>
<div class="stat"><div class="label">Additions</div><div class="value">+2,793</div><div class="sub">2,336 are <code>uv.lock</code></div></div>
<div class="stat"><div class="label">Reviewable surface</div><div class="value">~457</div><div class="sub">lines excl. lockfile</div></div>
<div class="stat"><div class="label">Production code</div><div class="value">+1</div><div class="sub">EOL fix in <code>__init__.py</code></div></div>
<div class="stat"><div class="label">Findings</div><div class="value" id="findings-total">—</div><div class="sub" id="findings-breakdown">—</div></div>
<div class="stat"><div class="label">Empirical checks</div><div class="value" id="checks-total">—</div><div class="sub">all claims source-verified</div></div>
<div class="stat"><div class="label">Praise</div><div class="value" id="praise-total">—</div><div class="sub">honest gotchas, scope discipline</div></div>
</div>
<section>
<h2>Headline actions</h2>
<ol class="top3">
<li><strong>Shard the bus by probe — one narrow <code>ObservableDict</code> per probe</strong> (<code>memory_bus</code>, <code>tasks_bus</code>, <code>watchdog_bus</code>, …). This matches NiceGUI's own pattern: <code>app.storage.user</code>, <code>app.storage.general</code>, <code>app.storage.tab</code> are <em>separate</em> ObservableDicts, not one global instance. Empirical benchmark (Theme F) shows per-write cost is linear in handlers-per-instance — sharding cuts cost proportionally and dissolves the frequency-mismatch problem (slow consumers attach to slow shards). Do <strong>not</strong> commit to one global <code>ObservableDict</code> before the next session.</li>
<li><strong>Copy NiceGUI's <code>[tool.ruff.lint]</code> + <code>[tool.mypy]</code> blocks verbatim into <code>pyproject.toml</code>.</strong> CLAUDE.md already says "match the contributing-to-nicegui skill standards" — those standards <em>are</em> NiceGUI's lint config. Two-minute change; immediately satisfies the claim. Also align the ruff floor between pyproject and pre-commit.</li>
<li><strong>Move <code>docs/sessions/</code> to a private staging repo.</strong> Line 4 of the current session prompt already commits a literal Linux home-directory path (<code>/home/&lt;user&gt;/.../nicegui-diagnostics/</code> shape) to a public repo. Future session prompts will only widen the leak surface. Replace with a one-line citation in the PR description. See the proposal in Theme D.</li>
</ol>
</section>
<section>
<h2>Empirical checks <span class="tag tag-info">source-verified, 2026-05-22</span></h2>
<p style="color: var(--muted);">Every load-bearing claim in this review checked against live source rather than memory.</p>
<table id="empirical-table">
<thead><tr><th>What we checked</th><th>How</th><th>Finding</th></tr></thead>
<tbody>
<tr>
<td>Does NiceGUI ship <code>py.typed</code>?</td>
<td><code>uvx --from nicegui python -c "(p / 'py.typed').exists()"</code>; <code>gh api commits?path=nicegui/py.typed</code></td>
<td>Yes. Added 2023-12-08 (commit <code>8d86a6e</code>). 3.10.0+ all ship it; verified on NiceGUI 3.12.1. mypy <code>additional_dependencies: ['nicegui']</code> delivers real type info.</td>
</tr>
<tr>
<td>Does NiceGUI (a library) commit <code>uv.lock</code>?</td>
<td><code>curl -I https://raw.githubusercontent.com/zauberzeug/nicegui/main/uv.lock</code></td>
<td>Yes (HTTP 200). The diagnostics package committing one is consistent with the parent project.</td>
</tr>
<tr>
<td>What lint/type config does NiceGUI use?</td>
<td><code>curl</code> NiceGUI's <code>pyproject.toml</code> + <code>.pre-commit-config.yaml</code></td>
<td><code>[tool.ruff.lint] select = ["I","E","W","B","F","UP","RUF","PL","Q"]</code> + explicit ignore list + <code>flake8-quotes</code> single-quote. <code>[tool.mypy] check_untyped_defs = true</code>. Pre-commit pins ruff <code>v0.14.5</code>. <strong>Copy verbatim — see Theme B.</strong></td>
</tr>
<tr>
<td>Does the current session prompt leak a filesystem path?</td>
<td><code>gh api .../contents/docs/sessions/...?ref=refs/pull/1/head | base64 -d</code></td>
<td>Yes, line 4 commits a literal <code>/home/&lt;user&gt;/.../nicegui-diagnostics/</code> path (exact string elided). <strong>See Theme D.</strong></td>
</tr>
<tr>
<td>What does complexipy's default threshold look like?</td>
<td>Read <code>complexipy/main.py</code> source</td>
<td>Typer default is <code>None</code> (no hard-coded number); README examples use 10. PG/this-repo's 15 is a deliberate choice, not "the default."</td>
</tr>
<tr>
<td>What is the actual cost of <code>ObservableDict</code> under load?</td>
<td>Benchmark script run under <code>uvx --from nicegui</code> on this machine; sources <code>nicegui/observables.py</code>, <code>events.py</code>, <code>binding.py</code> read</td>
<td>Per-write cost linear in handler count (0.31 µs at 0 handlers; 93 µs at 30 handlers; 309 µs at 100 handlers). NiceGUI's UI binding is poll-based at 100 ms — push semantics not required for live HUD. <strong>See Theme F.</strong></td>
</tr>
</tbody>
</table>
</section>
<section>
<h2>Theme A — PG inheritance <span class="tag tag-ok">mostly fine</span></h2>
<p style="color: var(--muted);">Brian standardising CI/tooling from PG into a new repo is the same pattern as evnchn copying NiceGUI's setup into personal projects — the maintainer's "shape A onto X, Y, Z" hygiene. Most of what looks like cargo-cult here is legitimate inheritance. One small suggestion below.</p>
<table>
<thead><tr><th>Sev</th><th>Where</th><th>Finding</th></tr></thead>
<tbody>
<tr>
<td class="sev"><span class="pill p-nit">Nit</span></td>
<td class="where">.pre-commit-config.yaml:27</td>
<td>
<code>exclude: '^(tests/|reference/)'</code> for complexipy — inherited from PG, fine. Optional: a one-line comment in the config explaining the rationale ("test code uses parametrize/fixtures that inflate cognitive complexity without being a code-quality problem") so a future contributor doesn't have to ask.
</td>
</tr>
</tbody>
</table>
</section>
<section>
<h2>Theme B — Lint/type stack: just copy NiceGUI's config <span class="tag tag-warn">action</span></h2>
<p style="background: var(--panel-2); border-left: 3px solid var(--soft); padding: 10px 14px; border-radius: 4px;">
CLAUDE.md already says "Match the contributing-to-nicegui skill standards." NiceGUI's <code>pyproject.toml</code> contains the canonical encoding of those standards — explicit <code>[tool.ruff.lint] select = […]</code>, an autopep8 line-length, <code>[tool.mypy]</code> with <code>check_untyped_defs = true</code>, plus a <code>flake8-quotes</code> block enforcing single-quote Python. Copy it. Don't design one from scratch. The findings below assume that path.
</p>
<table>
<thead><tr><th>Sev</th><th>Where</th><th>Finding</th></tr></thead>
<tbody>
<tr>
<td class="sev"><span class="pill p-high">High</span></td>
<td class="where">pyproject.toml:43 vs .pre-commit-config.yaml:4</td>
<td>
ruff version skew: <code>ruff&gt;=0.6</code> in pyproject, <code>v0.15.9</code> pinned in pre-commit. <code>uv run ruff</code> locally could be nine minor versions behind what pre-commit runs. NiceGUI itself pins <code>v0.14.5</code> in pre-commit and has no ruff floor in pyproject — pick that approach and align.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-med">Med</span></td>
<td class="where">pyproject.toml</td>
<td>
No <code>[tool.ruff.lint] select = [...]</code>. Default ruff = pyflakes (E/F) only. <strong>Direct lift from NiceGUI:</strong>
<pre style="background: var(--bg); border: 1px solid var(--border); border-radius: 4px; padding: 8px 10px; overflow-x: auto; font-size: 12px;"><code>[tool.ruff.lint]
select = ["I", "E", "W", "B", "F", "UP", "RUF", "PL", "Q"]
ignore = ["E501", "PLC0415", "PLR0911", "PLR0912",
"PLR0913", "PLR0915", "PLR2004"]
[tool.ruff.lint.flake8-quotes]
inline-quotes = "single"
multiline-quotes = "single"
docstring-quotes = "double"</code></pre>
Two-minute change; immediately satisfies the CLAUDE.md "match nicegui skill" claim.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-med">Med</span></td>
<td class="where">pyproject.toml</td>
<td>
No <code>[tool.mypy]</code> block. NiceGUI's is conservative but real:
<pre style="background: var(--bg); border: 1px solid var(--border); border-radius: 4px; padding: 8px 10px; overflow-x: auto; font-size: 12px;"><code>[tool.mypy]
python_version = "3.10"
check_untyped_defs = true</code></pre>
For a library with a typed public API ambition, you can go further with <code>disallow_untyped_defs = true</code> or <code>strict = true</code> — but at minimum, copy NiceGUI's baseline.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-med">Med</span></td>
<td class="where">CLAUDE.md:24</td>
<td>
<code>uv sync</code> in the "Commands" block installs runtime deps only. The next three lines (<code>pytest</code>, <code>ruff</code>, <code>mypy</code>) won't work on a fresh checkout. Either move dev deps to <code>[dependency-groups]</code> (PEP 735, auto-activated by uv) or document <code>uv sync --extra dev</code>.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-low">Low</span></td>
<td class="where">.pre-commit-config.yaml:16</td>
<td>
mypy <code>files: ^src/</code> excludes tests. For a library, the public API is exercised in tests — that's where call-site type checking pays off. Include tests.
</td>
</tr>
</tbody>
</table>
</section>
<section>
<h2>Theme C — <span style="color: var(--muted); font-weight: 400;">uv.lock in a library repo</span> <span class="tag tag-ok">no concern</span></h2>
<p style="color: var(--muted);">Originally flagged as "libraries shouldn't commit lockfiles." NiceGUI itself commits its <code>uv.lock</code> on <code>main</code> — the diagnostics package doing the same is consistent with the parent project. Nothing to do.</p>
</section>
<section>
<h2>Theme D — The "build by session prompt" pattern <span class="tag tag-info">meta-concern</span></h2>
<table>
<thead><tr><th>Sev</th><th>Where</th><th>Finding</th></tr></thead>
<tbody>
<tr>
<td class="sev"><span class="pill p-high">High</span></td>
<td class="where">docs/sessions/2026-05-22-initial-build.md:4</td>
<td>
<strong>Path leak:</strong> the file publicly commits a literal Linux home-directory path (<code>/home/&lt;user&gt;/.../nicegui-diagnostics/</code> shape — exact string elided here to avoid re-publishing it) on a public GitHub repo. Verified via <code>gh api .../contents/...?ref=refs/pull/1/head | base64 -d</code>. This is the first leak; subsequent session prompts will want to mention env vars, adjacent worktrees, and "what I just touched in <code>~/.worktrees/foo</code>" — same shape, larger surface each time. See the proposal below for the recommended fix (move session prompts to a private repo).
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-high">High</span></td>
<td class="where">CLAUDE.md:53-67</td>
<td>
"Design centre: the bus" calls itself <em>load-bearing</em>, then admits "the design docs don't yet reflect" it. A fresh reader of <code>00-vision.md</code> through <code>06-</code> will miss the priority. Promote it to <code>docs/design/08-bus-as-design-centre.md</code> before the next session opens a file.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-med">Med</span></td>
<td class="where">docs/sessions/2026-05-22-initial-build.md:27</td>
<td>
Specifying the exact error message string <code>RuntimeError("nicegui-diagnostics already installed")</code> in a session prompt is design-by-fiat. The string is a public-API choice. Let the implementing session propose it; review there.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-med">Med</span></td>
<td class="where">docs/sessions/2026-05-22-initial-build.md:22</td>
<td>
"Single-writer-per-key contract; concurrent writes are last-writer-wins (document this)." Misleading framing — <code>ObservableDict</code> isn't thread-safe; the relevant model is asyncio cooperative multitasking, not threads. <strong>But</strong> the watchdog probe in <code>07-pg-monkeypatch-inventory.md</code> §2 runs in a daemon thread and would write to the bus. That needs <code>loop.call_soon_threadsafe</code>, not LWW. Tighten before it becomes gospel.
</td>
</tr>
<tr>
<td class="sev"><span class="pill p-low">Low</span></td>
<td class="where">docs/sessions/2026-05-22-initial-build.md:38</td>
<td>
"Collector must … run synchronously in &lt;1 ms" is unenforceable in the code that will be written, and platform-dependent anyway. It will sit in a docstring and be ignored.
</td>
</tr>
</tbody>
</table>
<h3 style="margin-top: 22px;">Proposal: <code>docs/sessions/</code> → private repo</h3>
<p>The path leak above isn't a one-off — it's the trajectory of the pattern. Every session prompt will want to mention paths, environment, and adjacent-worktree state. Public repos are the wrong soil.</p>
<table>
<thead><tr><th>Decision</th><th>Reasoning</th></tr></thead>
<tbody>
<tr>
<td><strong>Drop <code>docs/sessions/</code> from the public repo entirely.</strong></td>
<td>Three problems at once: (1) token bloat (13 P0 items × ~127 lines each ≈ a session-prompt corpus the size of the rest of the project); (2) format is paste-into-Claude, not human-scannable; (3) <strong>secret-shape leak</strong> — Brian's home-dir path already committed publicly at line 4.</td>
</tr>
<tr>
<td><strong>Brian opens a private staging repo to hold the prompts.</strong></td>
<td>Path leaks, adjacent-worktree mentions, env-shape details are all fine in private. Public repo's audit trail gets a one-line citation: "built per session 2026-05-22, archived privately."</td>
</tr>
<tr>
<td><strong>Format: raw markdown for prompts; <code>renders/</code> subdir for Rich Render HTMLs.</strong></td>
<td>Two consumption modes, two formats. <em>Next-session input</em> wants raw <code>.md</code> (paste-into-Claude). <em>Retrospective browsing</em> ("what did I ship in May?") wants HTML dashboards — same Rich Render pattern this report uses. Don't force one format to serve both.</td>
</tr>
<tr>
<td><strong>Framing for the @-mention to Brian: lead with the leak, not the vibe.</strong></td>
<td>"line 4 commits a literal contributor home-directory path to a public repo" is concrete and hard to dismiss. "Your session prompts aren't browsable enough" is vibe-shaped and easy to dismiss.</td>
</tr>
</tbody>
</table>
</section>
<section>
<h2>Theme E — Smaller / line-level <span class="tag tag-info">nits + cleanups</span></h2>
<table>
<thead><tr><th>Sev</th><th>Where</th><th>Finding</th></tr></thead>
<tbody>
<tr><td class="sev"><span class="pill p-nit">Nit</span></td><td class="where">pyproject.toml:11,20</td><td>Both SPDX <code>license = "MIT"</code> and the OSI classifier — PEP 639 deprecates the classifier when SPDX is set. Drop the classifier.</td></tr>
<tr><td class="sev"><span class="pill p-low">Low</span></td><td class="where">pyproject.toml:13</td><td>Single author <code>evnchn</code> but the PR is two-Claude / two-human. Add Brian as co-author.</td></tr>
<tr><td class="sev"><span class="pill p-nit">Nit</span></td><td class="where">.pre-commit-config.yaml:43</td><td><code>check-added-large-files --maxkb=1000</code> won't catch the next 523KB <code>uv.lock</code> regen — the largest file in the repo. Either lower the cap or accept the lock recurring in diffs.</td></tr>
<tr><td class="sev"><span class="pill p-low">Low</span></td><td class="where">.pre-commit-config.yaml:30-33</td><td>gitleaks runs with default config. Vendored <code>reference/</code> source could contain example tokens. Consider a <code>.gitleaks.toml</code> allowlisting <code>reference/</code>.</td></tr>
<tr><td class="sev"><span class="pill p-low">Low</span></td><td class="where">CLAUDE.md:43-51</td><td>Sibling-repo map encodes Brian's local layout. Will rot the moment anyone clones elsewhere. Replace with URLs or flag "Brian-local."</td></tr>
<tr><td class="sev"><span class="pill p-low">Low</span></td><td class="where">CLAUDE.md:74 / pyproject.toml:39</td><td><code>structlog</code> is in <code>[project.optional-dependencies]</code> but the interval emitter (which uses it) is a P0 item. "Optional in the dep sense, required for one feature" — spell that out.</td></tr>
<tr><td class="sev"><span class="pill p-nit">Nit</span></td><td class="where">docs/design/07-pg-monkeypatch-inventory.md:24</td><td>Quoted PG code uses printf-style <code>%s/%d</code> in <code>structlog.debug()</code>. structlog idiomatic style is <code>logger.debug("event", key=value)</code>. When this lands in the package, do it the structlog way — don't copy PG's percent-style.</td></tr>
<tr><td class="sev"><span class="pill p-nit">Nit</span></td><td class="where">docs/design/07-pg-monkeypatch-inventory.md:80</td><td><code>features=["loop_watchdog", "stack_dump"]</code> — consider <code>Literal[...]</code> or an <code>Enum</code> for type-checked feature discovery (matches the typed-API ambition).</td></tr>
</tbody>
</table>
</section>
<section>
<h2>Theme F — Bus performance + dev UX <span class="tag tag-warn">empirical · shard by probe</span></h2>
<p style="background: var(--panel-2); border-left: 3px solid var(--bad); padding: 10px 14px; border-radius: 4px;">
<strong>Framing:</strong> PG is the laggy patient. The diagnostics tool is the doctor. If the doctor uses the patient's sus pattern, the doctor gets the patient's sickness. <strong>Don't accept "the bus is the design centre" without measurement.</strong> Below: numbers, architectural smells, and the empirically-supported conclusion (<strong>shard the bus by probe</strong>, matching NiceGUI's own multi-ObservableDict pattern).
</p>
<h3>F.1 — Benchmark numbers (Python 3.12, NiceGUI 3.12.1, M-series Mac)</h3>
<table>
<thead><tr><th>Scenario</th><th>Cost</th><th>Note</th></tr></thead>
<tbody>
<tr><td>Plain dict write (baseline)</td><td><code>0.013 µs/op</code></td><td>The reference point.</td></tr>
<tr><td><code>ObservableDict</code>, 0 handlers</td><td><code>0.31 µs/op</code></td><td>24× plain dict — the cost of the <code>last_modified</code> bookkeeping + change-handler check.</td></tr>
<tr><td><code>ObservableDict</code>, 1 handler</td><td><code>3.5 µs/op</code></td><td>+3.2 µs per handler — that's <code>events.handle_event</code> (introspect <code>expects_arguments</code>, set up <code>parent_slot</code>, exception wrap, sync/async check).</td></tr>
<tr><td><code>ObservableDict</code>, 30 handlers</td><td><code>93 µs/op</code></td><td>Linear in handler count. 300× plain dict.</td></tr>
<tr><td><code>ObservableDict</code>, 100 handlers</td><td><code>309 µs/op</code></td><td>1000× plain dict. Still sub-millisecond per write, but the slope is alarming.</td></tr>
<tr><td><code>dict(bus)</code> snapshot, 2000 flat keys</td><td><code>5.3 µs/op</code></td><td>Cheap. Snapshotting is not the bottleneck.</td></tr>
<tr><td>JSON serialise 90-key mixed bus</td><td><code>34 µs/op</code></td><td>Endpoint cost is negligible vs handler fan-out.</td></tr>
<tr><td>Nested-dict write (auto-wrap via <code>_observe</code>)</td><td>1.34× flat</td><td>Mild. <code>bus['memory'] = {'rss_mb': ...}</code> isn't free but isn't a footgun.</td></tr>
</tbody>
</table>
<h3 style="margin-top: 22px;">F.2 — The good news: v0.1 scale is fine</h3>
<p>~10 UI bindings, ~50 keys, watchdog at 20 Hz, slow probes at 0.03 Hz. Walking through the math:</p>
<ul class="tight">
<li>Watchdog: 20 writes/sec × 10 on_change handlers (if 10 subscribers exist) = 0.7 ms/sec overhead = <strong>0.07% of one CPU core.</strong> Fine.</li>
<li>Slow probes: negligible.</li>
<li>JSON endpoint: 34 µs per request. Fine.</li>
</ul>
<p><strong>You can ship v0.1 with ObservableDict and the numbers do not bite back.</strong> The smell is at the architecture level, not the scale level.</p>
<h3 style="margin-top: 22px;">F.3 — Smell 1: <code>on_change</code> is an unconditional firehose</h3>
<p>NiceGUI's <code>ObservableDict._handle_change</code> iterates every registered handler on every mutation. <strong>There is no per-key subscription.</strong> A subscriber that only cares about <code>memory_rss_mb</code> still runs when <code>client_42_age_s</code> changes — it has to defend itself by inspecting the event and discarding ~99% of calls.</p>
<p>Reading the source (<code>nicegui/observables.py</code>) confirms it:</p>
<pre style="background: var(--bg); border: 1px solid var(--border); border-radius: 4px; padding: 8px 10px; overflow-x: auto; font-size: 12px;"><code>def _handle_change(self) -&gt; None:
self.last_modified = time.time()
for handler in self.change_handlers: # &lt;-- every handler, every time
events.handle_event(handler, ObservableChangeEventArguments(sender=self))</code></pre>
<p>The <code>ObservableChangeEventArguments</code> carries only <code>sender: ObservableCollection</code> — no changed-key, no value, no previous-value. <strong>Per-sub-collection filtering is available cheaply</strong> via the <code>sender</code> field: the parent-chain walk sets <code>sender</code> to the specific nested <code>ObservableDict</code> that mutated, so a handler can tell <em>which</em> sub-collection changed. <strong>Per-key filtering within a sub-collection</strong> still needs diffing or an external pubsub layer. <em>Implication:</em> narrow <code>ObservableDict</code>s work fine — one giant global one does not.</p>
<h3 style="margin-top: 22px;">F.4 — Smell 2: frequency mismatch is a code-smell, not a feature <span class="tag tag-bad" style="font-size:11px;">user finding, blunt</span></h3>
<p style="background: var(--panel-2); padding: 10px 14px; border-radius: 4px; font-style: italic;">
"If one source spams per-second and one source sends periodically per-minute, if I observe to the <code>ObservableDict</code> I don't wanna write an observer which ignores 99.9% of the time either. That's straight up dumb." — evnchn, 2026-05-22
</p>
<p>Concrete: the planned structlog interval emitter wants "every 30s, snapshot the current state." With <code>on_change</code> as the only subscription primitive, it would either:</p>
<ol class="tight">
<li>Subscribe to <code>on_change</code> and ignore 599 out of every 600 calls (watchdog at 20 Hz × 30s = 600 wakeups per intended emission). Wasteful + ugly defensive code.</li>
<li>Or NOT subscribe — instead, run on its own <code>ui.timer(30, ...)</code> and call <code>collect_snapshot()</code> on tick. Clean, but then <strong>what is the bus doing for this consumer?</strong> Nothing. The bus is just a plain dict at that point.</li>
</ol>
<p>Option 2 is correct. Which means the "everyone subscribes to the bus" framing dies for any consumer whose natural cadence differs from the highest-frequency producer.</p>
<h3 style="margin-top: 22px;">F.5 — Smell 3: NiceGUI's UI binding is ALREADY poll-based</h3>
<p>Here's the load-bearing finding that weakens the whole bus framing. Reading <code>nicegui/binding.py</code>:</p>
<pre style="background: var(--bg); border: 1px solid var(--border); border-radius: 4px; padding: 8px 10px; overflow-x: auto; font-size: 12px;"><code>async def refresh_loop() -&gt; None:
...
if core.app.config.binding_refresh_interval is None:
core.app.config.binding_refresh_interval = 0.1
while True:
_refresh_step() # walks every active_link, reads source, pushes if changed
await asyncio.sleep(core.app.config.binding_refresh_interval)</code></pre>
<p>NiceGUI's <code>bind_text_from(bus, 'key')</code> does <strong>not</strong> register an <code>on_change</code> handler on the bus. It registers in the <code>bindings</code> defaultdict and is polled by <code>refresh_loop</code> at <code>binding_refresh_interval</code> (default 100 ms). <strong>The "live HUD" is already a 10 Hz poll, not a push.</strong></p>
<p style="background: var(--panel-2); padding: 10px 14px; border-radius: 4px;"><strong>Implication:</strong> if the live HUD is the load-bearing justification for the bus, and the live HUD is poll-based anyway, then a <strong>plain dict + the same NiceGUI binding</strong> gives you the live HUD <em>without</em> the on_change firehose. The CLAUDE.md "bus as design centre" argument needs an asterisk: it's the design centre for HTTP-snapshot + structlog + invariant composition, but the live HUD doesn't actually require <code>ObservableDict</code>.</p>
<h3 style="margin-top: 22px;">F.6 — Re-examining who actually needs push subscription</h3>
<table>
<thead><tr><th>Consumer</th><th>Natural cadence</th><th>Needs push?</th><th>Why</th></tr></thead>
<tbody>
<tr><td>UI live HUD (<code>ui.diagnostics_view</code>)</td><td>10 Hz</td><td>NO</td><td>NiceGUI binding is poll-based at 100 ms already.</td></tr>
<tr><td>HTTP <code>/_nicegui/diagnostics</code></td><td>On request</td><td>NO</td><td>Pure poll — request comes in, return <code>dict(bus)</code>.</td></tr>
<tr><td>structlog interval emitter</td><td>Per 30s timer</td><td>NO</td><td><code>ui.timer(30, emit_snapshot)</code> is cleaner than filtering 599/600 wakeups.</td></tr>
<tr><td>Prometheus exporter (future)</td><td>15s scrape</td><td>NO</td><td>Scrape is poll-based by definition.</td></tr>
<tr><td>Invariant probe (contextvar identity)</td><td>Per relevant write</td><td>MAYBE</td><td>Only one that benefits from reactive subscription — and only on a small key set, where firehose pain is highest.</td></tr>
<tr><td>Future "alert on threshold cross"</td><td>Per relevant write</td><td>MAYBE</td><td>Same shape as invariants.</td></tr>
</tbody>
</table>
<p><strong>Conclusion:</strong> 4 of 6 consumer types want polling, not subscription — they don't care about the bus primitive, they just need <code>dict(bus)</code>-style snapshotting. The 2 that want reactive subscription want it <em>scoped narrowly</em> — to "this probe's state," not "every key in the universe." That's exactly what <strong>sharding by probe</strong> delivers: each push subscriber attaches to one narrow ObservableDict, gets reactivity for the keys it cares about, and pays nothing for unrelated probe writes. F.7 below ranks this against the alternatives.</p>
<h3 style="margin-top: 22px;">F.7 — Alternative primitives, ranked</h3>
<table>
<thead><tr><th>Primitive</th><th>What it offers</th><th>Downside</th></tr></thead>
<tbody>
<tr style="background: rgba(63,185,80,0.07);">
<td><strong>Shard <code>ObservableDict</code> by probe</strong> &nbsp;<span class="pill p-ok">RECOMMENDED</span></td>
<td>One narrow <code>ObservableDict</code> per probe: <code>memory_bus</code>, <code>tasks_bus</code>, <code>watchdog_bus</code>, <code>lifecycle_bus</code>. Subscribers attach only to shards whose cadence + content they care about. <strong>Matches NiceGUI's own pattern</strong> (<code>app.storage.user</code> / <code>.general</code> / <code>.tab</code> are separate ObservableDicts). Per-write cost is linear in handlers-per-instance — sharding cuts cost proportionally vs the monolithic version. Frequency mismatch dissolves at the shard boundary: structlog emitter polls all shards on its own 30s timer; watchdog ticks don't wake it.</td>
<td>"Which shard does this key live on?" needs a registry — but <code>register_collector</code> was planned anyway, so this falls out for free. Cross-shard invariants need explicit multi-subscription (fine — there are few, and <code>sender</code> identifies the firing shard).</td>
</tr>
<tr>
<td>Plain dict + version counter</td>
<td>Cheapest possible. Producers write to <code>bus: dict</code>; bump <code>bus_version</code> int. Consumers poll on their own schedule.</td>
<td>Throws away the reactive primitive entirely — the 2 push-shaped consumers (invariants, alerts) lose reactivity even where they could benefit.</td>
</tr>
<tr>
<td>Plain dict + per-key pubsub layer</td>
<td><code>bus.subscribe('memory_*', handler)</code>. Glob/regex patterns; handler only fires on matches.</td>
<td>~50 LOC of custom pubsub plumbing vs reusing NiceGUI's primitive. Sharding gets you 80% of the same benefit without the custom code.</td>
</tr>
<tr>
<td>Two-tier: hot dict + cold ObservableDict</td>
<td>High-freq keys in plain dict; low-freq in ObservableDict for invariant subscription.</td>
<td>Two structures to teach. Subsumed by sharding (the "hot" tier becomes its own ObservableDict shard with no subscribers).</td>
</tr>
<tr>
<td>Status quo: one global <code>ObservableDict</code></td>
<td>Works at v0.1 scale. Reuses NiceGUI machinery.</td>
<td>Every consumer pays for every producer's frequency. Firehose shape is wrong-shaped at PG-scale and dev-UX-ugly even at v0.1. <strong>Not recommended.</strong></td>
</tr>
</tbody>
</table>
<h3 style="margin-top: 22px;">F.8 — Recommendation</h3>
<ol class="tight">
<li><strong>v0.1: shard the bus by probe.</strong> One narrow <code>ObservableDict</code> per probe — <code>memory_bus</code>, <code>tasks_bus</code>, <code>watchdog_bus</code>, <code>lifecycle_bus</code>, etc. This matches NiceGUI's own pattern (<code>app.storage.user</code>/<code>.general</code>/<code>.tab</code> are separate ObservableDicts, not one global). Uses the primitive the way the parent project does.</li>
<li><strong>Compose snapshots via a registry.</strong> The already-planned <code>register_collector(name, callable)</code> generalises to <code>register_bus(name, bus)</code>. <code>collect_snapshot()</code> becomes <code>{name: dict(b) for name, b in registered_buses.items()}</code>. Adding a probe = registering a new shard; it shows up in all consumers automatically.</li>
<li><strong>Subscribers attach to the shard whose cadence matches their needs.</strong>
<ul class="tight" style="margin-top: 4px;">
<li>UI live HUD → NiceGUI's existing poll-based binding against whichever shards display elements care about (binding is poll-based at 100ms regardless of the underlying object — see F.5).</li>
<li>HTTP endpoint, structlog interval emitter, Prometheus scraper → don't subscribe at all; walk all shards on own cadence.</li>
<li>Invariant probes, threshold alerts → subscribe to just the specific shards they need. Per-write cost stays small because handler count per shard stays small.</li>
</ul>
</li>
<li><strong>Rewrite CLAUDE.md's "Design centre: the bus" framing.</strong> The load-bearing thing is <em>"the bus pattern, instantiated as one ObservableDict per probe"</em> — not <em>"the one global ObservableDict."</em> Words matter for the next session: the singular framing leads to the firehose; the plural framing leads to the NiceGUI-shaped design.</li>
<li><strong>Measure under PG-realistic load before PG adopts.</strong> 100+ keys across ~10 shards, 50+ UI elements bound, watchdog at 20 Hz. If shard sizes stay small (≤10 handlers each), overhead should be &lt;0.5% of one core. If a shard grows past that, split it further.</li>
</ol>
<h3 style="margin-top: 22px;">F.9 — Pluggable eviction (mix-and-match, not one true policy)</h3>
<p>Each shard's keys grow forever without an eviction policy. <strong>There is no single right policy</strong> — what works for high-frequency watchdog ticks (drop after seconds) is wrong for per-client lifecycle counters (keep until client disconnects). Ship eviction as <em>composable evictors</em>, one or more per shard, that users mix and match:</p>
<table>
<thead><tr><th>Evictor</th><th>Drops a key when</th><th>Good for</th></tr></thead>
<tbody>
<tr>
<td><strong>Time-based</strong></td>
<td>Key's <code>last_modified</code> is older than <code>max_age_s</code></td>
<td>Sliding-window metrics; high-freq probes where stale values are uninteresting (watchdog block timestamps, last-request latency).</td>
</tr>
<tr>
<td><strong>Disk-based / spill</strong></td>
<td>Key's <code>last_modified</code> is older than <code>cold_after_s</code> — flush to disk + remove from memory; on read, load back if requested</td>
<td>Long-tail metrics you want kept but rarely read (per-client lifecycle history before disconnect, post-mortem snapshots).</td>
</tr>
<tr>
<td><strong>Size-based / LRU</strong></td>
<td>Shard exceeds <code>max_keys</code> — drop the least-recently-used</td>
<td>Per-client per-tab dicts where the client count is unbounded and bounded memory matters more than completeness.</td>
</tr>
<tr>
<td><strong>Event-triggered</strong></td>
<td>External callback fires (e.g. <code>app.on_disconnect(client)</code> → drop <code>client_&lt;id&gt;_*</code>)</td>
<td>Per-client keys with a known lifetime. Most precise; needs the event source to exist.</td>
</tr>
</tbody>
</table>
<p>A shard registers zero or more evictors. <code>shard.evictors = [TimeEvictor(60), SizeEvictor(max_keys=1000)]</code> — both run; either can drop a key. Users compose what matches their probe's data shape.</p>
<h3 style="margin-top: 22px;">F.10 — Implementation decisions</h3>
<table>
<thead><tr><th>Question</th><th>Decision (evnchn, 2026-05-22)</th><th>Notes</th></tr></thead>
<tbody>
<tr>
<td>Per-client key layout: flat or nested?</td>
<td><strong>Nested.</strong> <code>clients_bus[client_id] = {'memory_kb': …, 'age_s': …, …}</code>.</td>
<td>Falls out cleanly with sharding: one <code>clients_bus</code> shard, each client a top-level key with an inner dict that <code>_observe</code> auto-wraps as an <code>ObservableDict</code>. Eviction is <code>del clients_bus[id]</code> on disconnect. Iteration is the natural <code>for cid, state in clients_bus.items()</code>.</td>
</tr>
<tr>
<td>Schema registry — how is the key set documented and versioned?</td>
<td><strong>The released version <em>is</em> the schema.</strong> Each minor version's published key set is the living standard. <strong>v0:</strong> add/rename/remove keys freely. <strong>v1+:</strong> additions OK in any minor; removals require deprecation in one minor version and removal in a later one. v2 may not remove a v1 key that wasn't deprecated in v1.x.</td>
<td>No runtime introspection registry required — the changelog + the version pin is the contract. Cheap to enforce: deprecated keys can emit a one-time <code>DeprecationWarning</code> on first write, and a checklist item gates the minor bump.</td>
</tr>
<tr>
<td><code>make_metric_bus()</code> helper from design doc 02 — keep?</td>
<td><strong>Drop.</strong> Why "make" when you just <em>have</em> an <code>ObservableDict</code>? Sharding subsumes the original use case. If a specific call site later needs a factory, add it then.</td>
<td>Not forbidding <code>make_*</code> in general — just not committing one upfront. The shard-per-probe pattern doesn't need it.</td>
</tr>
<tr>
<td>Thread-safe write helper for the watchdog probe?</td>
<td><strong>Watchdog-specific concern, not a general bus API question.</strong> Most probes run inside the asyncio loop, single-thread, no helper needed. The watchdog <em>must</em> run in a daemon thread because its job is detecting <em>loop blockage</em> — an asyncio task would itself be blocked at the moment there's something to detect. The thread is load-bearing for the feature, not an over-architectural choice.</td>
<td>Defer the helper design to the watchdog-probe implementation session. Don't bake it into the bus surface; let it live next to the daemon thread that needs it. If the watchdog feature gets dropped from P0, this question dissolves entirely.</td>
</tr>
</tbody>
</table>
<p style="background: var(--panel-2); padding: 12px 16px; border-left: 3px solid var(--ok); border-radius: 4px; margin-top: 16px;">
<strong>Hot-take bottom line:</strong> the bus framing is right in spirit (one source of truth, decoupled consumers). The <em>implementation</em> needs to follow NiceGUI's own pattern — multiple narrow ObservableDicts, one per probe, not one global one. The benchmark confirms it: cost is linear in handlers-per-instance, so sharding cuts that cost proportionally and dissolves the frequency-mismatch problem at the shard boundary. The doctor-inheriting-patient's-sickness failure mode is averted not by abandoning <code>ObservableDict</code> but by <strong>using it the way NiceGUI uses it.</strong>
</p>
</section>
<section>
<h2>What's good <span class="tag tag-ok">don't lose these</span></h2>
<div class="cols" id="praise-cards">
<div class="card">
<h3>Commit hygiene</h3>
<p>Three commits split by concern (dev env / CLAUDE.md / docs). Each independently reviewable. The co-author trailer matches the actual contributor.</p>
</div>
<div class="card">
<h3>Scope discipline</h3>
<p>Out-of-scope lists in <em>both</em> <code>07-pg-monkeypatch-inventory.md</code> and the session prompt. Explicit deferrals (Stytch, PgBouncer, CRDT, bash watchdog stay in PG; HTTP endpoint, stack dump, view element deferred from session 1).</p>
</div>
<div class="card">
<h3>Critical thinking in Gotchas</h3>
<p>Port 9999 collision call-out; "private prior-art was removed from production after 5 months — could mean opt-in, could mean not worth the runtime cost"; bug 3 unreproduced for 6+ weeks. This is the <em>right</em> mode for design docs.</p>
</div>
<div class="card">
<h3>Vendored-source hygiene</h3>
<p><code>tool.ruff.extend-exclude = ["reference/", "docs/"]</code> keeps upstream bit-identical for diff-against-upstream. Well chosen.</p>
</div>
<div class="card">
<h3>README provenance</h3>
<p>Clean acknowledgement table tying each bug-find to the probe technique that found it. Reads as engineering memoir, not marketing.</p>
</div>
</div>
</section>
<details>
<summary>Methodology / what was actually reviewed</summary>
<ul class="tight">
<li>Cloned <code>evnchn-nicegui/nicegui-diagnostics</code>, checked out PR #1 head.</li>
<li>Read all 7 substantive files end-to-end: <code>.gitignore</code>, <code>.pre-commit-config.yaml</code>, <code>CLAUDE.md</code>, <code>pyproject.toml</code>, <code>docs/design/07-pg-monkeypatch-inventory.md</code>, <code>docs/sessions/2026-05-22-initial-build.md</code>, <code>src/nicegui_diagnostics/__init__.py</code>. Cross-referenced <code>README.md</code> for stated intent.</li>
<li><strong>Not</strong> reviewed line-by-line: <code>uv.lock</code> (auto-generated, 2336 lines).</li>
<li>NiceGUI sources read directly for the bus analysis: <code>nicegui/observables.py</code>, <code>nicegui/events.py</code>, <code>nicegui/binding.py</code>. Benchmark run under <code>uvx --from nicegui</code> on this machine (Python 3.12, NiceGUI 3.12.1, M-series Mac).</li>
<li><strong>Not</strong> consulted: PromptGrimoireTool source. Claims about PG patterns are based on what this PR documents about PG, not direct inspection.</li>
</ul>
</details>
<footer>
nicegui-diagnostics PR #1 review &middot; 2026-05-22 &middot; reviewer: Claude Opus 4.7 (evnchn's instance) with co-review by Evan
</footer>
</div>
<script>
// Auto-count findings, empirical checks, and praise cards from the DOM.
// Counts only severity pills that live inside finding-row <td class="sev"> cells
// (so the legend at the top is excluded).
(function () {
function count(sel, root) {
return (root || document).querySelectorAll(sel).length;
}
var high = count('td.sev .pill.p-high');
var med = count('td.sev .pill.p-med');
var low = count('td.sev .pill.p-low');
var nit = count('td.sev .pill.p-nit');
var total = high + med + low + nit;
var checks = count('#empirical-table tbody tr');
var praise = count('#praise-cards .card');
function set(id, text) {
var el = document.getElementById(id);
if (el) el.textContent = text;
}
set('findings-total', String(total));
set('findings-breakdown',
high + ' high · ' + med + ' med · ' + low + ' low · ' + nit + ' nit');
set('checks-total', String(checks));
set('praise-total', String(praise));
})();
</script>
</body>
</html>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment