Phase 6: bump livetemplate v0.8.16 → v0.10.0; migrate literate examples to Subscribe/Publish

[adnaan]

Phase 6 of the broadcast-redesign wave (livetemplate#415) — tinkerdown side. Coordinated with livetemplate#430.

Scope (post-rebase)

The Phase-6 worktree branched off pre-v0.9.0 SHA 751297a, but origin/main has since advanced via tinkerdown#278's pin bump to v0.9.0. After rebase onto current main, this PR's net change is:

• go.mod long-jump: livetemplate v0.9.0 → v0.10.0 (chore: bump livetemplate to v0.9.0 #278 already covered the v0.8.16 → v0.9.0 step). Crosses the v0.10.0 BroadcastAction removal. The earlier v0.9.0 release crossed the Sync() auto-dispatch removal (livetemplate#406) — the 2 literate examples never used Sync(), so only the BroadcastAction migration is needed in practice.
• Both literate examples migrated: counter-include, linked-include. Each gains a Mount-side ctx.Subscribe(ctx.SelfTopic()) (with error-propagation per Claude review feedback — safe-by-default for readers who copy the pattern with a developer topic), swaps 3 BroadcastAction call sites to ctx.Publish(ctx.SelfTopic(), ...), and gets matching index.md + main.go comment rewrites.

All 13 internal packages green; 3 root-package E2E tests pass once the tinkerdown binary is pre-built (make build) — pre-existing test-infra detail, not introduced here.

🤖 Generated with Claude Code

[claude]

Code Review — Phase 6: Subscribe/Publish migration

Overview

Clean, focused migration PR. The mechanical swap from BroadcastAction → Subscribe + Publish is consistent across both literate examples, comments and docs are updated, and the go.mod/go.sum bump to v0.10.0 looks correct. No concerns blocking merge, but a few things worth noting.

---

Issues / Suggestions

1. PR description says baseline is v0.8.16, but go.mod diff shows v0.9.0 → v0.10.0

The PR body describes this as a "long-jump: v0.8.16 → v0.10.0", but the actual diff starts from v0.9.0. Minor, but could confuse anyone bisecting history later. Worth correcting the description or adding a note that v0.9.0 was already landed separately.

2. Silently discarding ctx.Subscribe error

_ = ctx.Subscribe(ctx.SelfTopic())

The comment justifies this (SelfTopic() is ACL-exempt, Subscribe always succeeds for it), which is a fair explanation for example code. However, if this pattern gets copied into production apps and the library's ACL guarantee ever changes, the error would be silently swallowed. Consider either:

• Keeping the discard but adding a //nolint or a very explicit note: // error is always nil for SelfTopic per livetemplate#415
• Or returning the error: if err := ctx.Subscribe(ctx.SelfTopic()); err != nil { return s, err } — makes the guarantee explicit and safe-by-default for readers copying the pattern.

3. Minor comment inconsistency between the two examples

literate-counter-include/counter.go Mount has this extra paragraph not present in literate-linked-include/counter.go:

// Dispatched actions reach this Mount too — but Subscribe is idempotent
// per-connection so re-Mounts are no-ops.

If the examples are meant to be independently copy-pasteable, they should be in sync. If literate-counter-include is intentionally more detailed (as a "first" tutorial), that's fine — just flagging the divergence.

4. Awkward line-break in main.go comment (both examples)

// `ctx.Publish(ctx.SelfTopic(), ...)` from any tab/embed reaches every
// other subscribed peer. Real apps use a per-user authenticator; here a constant groupID is

The phrase "here a constant groupID is" at the end of that line reads oddly because the sentence continues on the next line. This was likely pre-existing prose that got partially updated. Not blocking, but easy to tighten:

// `ctx.Publish(ctx.SelfTopic(), ...)` from any tab/embed reaches every other
// subscribed peer. Real apps use a per-user authenticator; the constant groupID here is

---

What looks good

• The two-step explanation in literate-linked-include/index.md ("Without the Subscribe, no peer registers as a receiver; without the Publish, no fan-out happens") is clear and pedagogically useful.
• go.sum hashes are consistent with the go.mod change — old v0.9.0 entries removed, new v0.10.0 entries added.
• No Sync() migration needed (correctly identified in the PR description — neither example used it).
• The recursion-guard note in the Increment/Decrement comments is a good reminder for readers.

---

Verdict

Approve with minor suggestions. Items 1 and 3 are worth a follow-up; item 2 is a judgment call for example code. Nothing here blocks the merge.

[claude]

Code Review

Overview

Clean migration of both literate examples from the removed BroadcastAction API to the new Subscribe/Publish pattern, coordinated with livetemplate#430. The dependency bump (v0.9.0 → v0.10.0) and the three action-handler swap sites are all consistent. The documentation prose updates are accurate and well-written.

---

Bug: Stale line-number references in both index.md files

This is the main issue. Adding the 18-line Mount function shifted all subsequent lines in counter.go, but the code-include directives were not updated.

literate-counter-include/index.md (line 32):

```go include="./_app/counter.go" lines="13-35" highlight="20"

After the PR, counter.go line 13 is now the first line of the Mount doc-comment. lines="13-35" renders the Mount function + just the Increment comment (lines 31–35), cutting off before the function body. highlight="20" lands on // is ACL-exempt today (livetemplate#415, v0.10.0)… inside the Mount block — not on the Publish call the surrounding text is describing (which is now at line 38).

Suggested fix:

-```go include="./_app/counter.go" lines="13-35" highlight="20"
+```go include="./_app/counter.go" lines="24-52" highlight="38"

literate-linked-include/index.md (line 22):
Same issue — lines="13-35" also needs updating here.

---

Minor: Mount body can be simplified

// current
func (c *CounterController) Mount(s Counter, ctx *livetemplate.Context) (Counter, error) {
    if err := ctx.Subscribe(ctx.SelfTopic()); err != nil {
        return s, err
    }
    return s, nil
}

// idiomatic
func (c *CounterController) Mount(s Counter, ctx *livetemplate.Context) (Counter, error) {
    return s, ctx.Subscribe(ctx.SelfTopic())
}

The two-variable form in the PR is readable, and for a literate example that deliberately shows the pattern step-by-step it may be intentional — but worth considering since the comment block already explains the error-propagation rationale.

---

Worth verifying: ctx.Publish error return

The three action handlers call ctx.Publish(...) without checking a return value. The Mount comment argues specifically for propagating the Subscribe error to be safe-by-default for readers who copy the pattern with developer topics. If ctx.Publish also returns an error in the v0.10.0 API, the same reasoning applies and the handlers should propagate it too. If it's void, no change needed.

---

Positive notes

• All three action handlers (Increment, Decrement, Reset) are migrated consistently — no missed call sites.
• The Mount comment is unusually good for a literate example: it explains why Subscribe is required, that it's idempotent across re-Mounts, and why error propagation is kept even though SelfTopic() is ACL-exempt. Readers who copy this into real apps will do the right thing.
• go.sum hashes are updated correctly.
• main.go comment in both examples is updated to match the new API name.

---

Summary: The stale lines= / highlight= directives in the two index.md files are the only correctness issue — they will render the wrong code block to readers of the literate tutorial. Everything else is clean.

[claude]

Code Review — Phase 6: bump livetemplate v0.9.0 → v0.10.0; migrate Subscribe/Publish

Overview

This PR is a clean, focused migration of the two literate examples (literate-counter-include, literate-linked-include) away from the removed BroadcastAction API to the new Subscribe/Publish two-step model, coupled with the required go.mod/go.sum dependency bump to v0.10.0. The scope is appropriately tight and the PR description is well-written.

---

What's Good

• Error propagation is handled correctly throughout. Every ctx.Subscribe and ctx.Publish call checks and returns errors. The rationale in the comment (safe-by-default for readers who copy the pattern with a developer topic) is sound.
• Both examples migrated consistently. The two counter.go files end up byte-identical (same git blob hash 05bcf64), which is the right outcome for two examples that are intentionally parallel.
• index.md docs updated in sync. The prose explanation of the Subscribe+Publish two-step is clearer than the old single-step broadcast explanation.
• lines= ranges in literate fences updated correctly (13-35 → 13-59) to match the 24-line growth from the new Mount method.

---

Issues / Suggestions

1. go.sum — identical /go.mod hash across v0.9.0 and v0.10.0 (worth verifying)

-github.com/livetemplate/livetemplate v0.9.0/go.mod  h1:GMvZKyPUq8LSGfgD3pftKOHa6v+I+RDYyff2mNjeAYs=
+github.com/livetemplate/livetemplate v0.10.0/go.mod h1:GMvZKyPUq8LSGfgD3pftKOHa6v+I+RDYyff2mNjeAYs=

The /go.mod hash is the same for both versions. This is possible if the upstream go.mod file itself didn't change between releases, but it's unusual for a minor version that removes a public API (BroadcastAction). Worth a quick go mod verify or comparing against a fresh go get github.com/livetemplate/livetemplate@v0.10.0 to confirm go sum wasn't accidentally hand-edited.

2. State divergence when Publish errors in action handlers

In Increment/Decrement/Reset, the state mutation happens before the Publish call:

s.Count++
if err := ctx.Publish(ctx.SelfTopic(), "Increment", nil); err != nil {
    return s, err   // mutation IS committed locally, fan-out silently fails
}

If Publish returns an error, the current client's state advances but peers receive nothing. The framework likely treats this as a hard connection error (triggering reconnect/re-mount), so in practice the divergence is transient — but a short comment noting this behaviour would make the example safer to copy into production code where the implication is less obvious.

3. Mount comment block is longer than it needs to be for an example

The 10-line block doc on Mount is good for a tutorial, but it references an internal issue number (livetemplate#415) that external readers can't access. Either link to the public changelog/release notes for v0.10.0 or drop the issue reference. The "safe-by-default" rationale is the valuable part — keep that.

4. No explicit test for the new Mount path

The PR description notes the existing 13-package suite and 3 E2E tests are green, which is fine. But Mount is now a required lifecycle hook for fan-out to work — if it's ever accidentally omitted, things silently break (no subscribe = no peers). A single E2E test that asserts cross-tab count sync would make that regression visible. Understood if it's deferred, but worth tracking.

---

Minor nits

• literate-linked-include/index.md line 37: "every other connected client that subscribed" — "that subscribed" is slightly redundant given the preceding sentence already explains the Subscribe step. Optional tightening.
• The CounterController struct has no fields. Mount (and all handlers) take a pointer receiver *CounterController — fine, just noting it's consistent.

---

Summary

Approve with minor follow-ups. The core migration is correct, error-safe, and well-documented. The go.sum hash coincidence is the only item I'd verify before merging; the rest are documentation/test suggestions that can be addressed in a follow-up.

[claude]

Code Review — Phase 6: bump livetemplate v0.8.16 → v0.10.0; migrate literate examples to Subscribe/Publish

Overview

This PR migrates both literate examples (literate-counter-include, literate-linked-include) away from the removed BroadcastAction API to the new Subscribe/Publish model introduced in livetemplate v0.10.0. The diff is clean and mechanical — every BroadcastAction call site is replaced, a Mount method is added for the subscription opt-in, and docs/comments are updated to match.

---

Code Quality & Style

Strengths:

• Error propagation is consistent throughout: Mount, Increment, Decrement, and Reset all properly return errors rather than discarding them.
• The rationale for propagating Mount's Subscribe error — even though SelfTopic() is currently ACL-exempt — is sound: it keeps the example safe-to-copy for readers who might use a non-exempt topic. This is good API teaching hygiene.
• Both examples are updated in lockstep; no drift between the two.

Minor concern — comment verbosity:
The new Mount and Increment doc comments run to multi-paragraph blocks. For literate/tutorial code this is defensible (readers are meant to learn from them), but the Increment comment in particular mixes two distinct concerns:

1. The normal flow (mutate → publish).
2. The failure-mode tradeoff (mutate-first vs. publish-first).

Consider splitting the failure-mode discussion into a separate // Note: block or moving it to index.md prose, so the handler comment stays scannable for readers who just want the happy path.

---

Potential Bugs / Issues

Mutate-before-publish ordering (acknowledged, but worth flagging explicitly):

s.Count++
if err := ctx.Publish(ctx.SelfTopic(), "Increment", nil); err != nil {
    return s, err  // local state already mutated; peers not notified
}

The PR comment correctly documents that this is a known tradeoff with the recursion guard. No action required, but since this is tutorial code that readers will copy, it's worth ensuring index.md also calls out this limitation — currently only the Go comment does. A one-sentence callout in the prose would help readers who only skim the Markdown.

Mount returns unmodified state — correct but subtle:

func (c *CounterController) Mount(s Counter, ctx *livetemplate.Context) (Counter, error) {
    if err := ctx.Subscribe(ctx.SelfTopic()); err != nil {
        return s, err
    }
    return s, nil
}

This is correct behavior (Mount here is purely a side-effect, no state change). No bug, but since this is the first time readers see a Mount that returns state unchanged, a one-line comment on the return ("state is unchanged; subscribe is the only effect") would prevent confusion.

---

go.sum observation

-github.com/livetemplate/livetemplate v0.9.0/go.mod h1:GMvZKyPUq8LSGfgD3pftKOHa6v+I+RDYyff2mNjeAYs=
+github.com/livetemplate/livetemplate v0.10.0/go.mod h1:GMvZKyPUq8LSGfgD3pftKOHa6v+I+RDYyff2mNjeAYs=

The go.mod hash for v0.10.0 is identical to v0.9.0. This is valid if the dependency's own go.mod file didn't change between versions (only source files did), but it's worth a double-check to confirm go mod verify passes cleanly — it would catch any hash mismatch.

---

Test Coverage

• 13 internal packages + 3 root E2E tests are noted as passing.
• No new unit tests for Mount — acceptable for tutorial/example code.
• The E2E tests requiring a pre-built binary (make build) is a pre-existing infra detail as noted, but worth documenting in a top-level CONTRIBUTING.md or README if not already there, since it's a non-obvious prerequisite for reviewers running CI locally.

---

Security

No concerns. SelfTopic() scopes fan-out to the authenticated session, so there's no cross-session data leakage introduced by this change.

---

Summary

The migration is correct, well-reasoned, and the error-propagation choices are sound. Main suggestions:

1. Add a brief mutate-before-publish caveat in index.md (not just in the Go comment) so readers skimming the tutorial understand the tradeoff.
2. Verify go mod verify passes to confirm the matching go.mod hash is expected.
3. Optional: slim the multi-paragraph Increment comment or split the failure-mode note from the main description.

Otherwise, LGTM.

[adnaan]

Round-4 review reads as no-functional-issue + 3 cosmetic refinements. Per CLAUDE.md bot review convergence — two consecutive rounds of cosmetic-only feedback is the convergence signal — stopping pushes and addressing via reply.

"Comment verbosity in Increment": This is a literate-tutorial example; the failure-mode tradeoff is precisely the kind of decision a reader copying the pattern into production needs to see at the code site, not buried in the surrounding prose. Splitting it into the index.md would mean readers who only skim the Go file (the "give me the pattern" mode) miss the rationale. Holding the verbose comment.

"index.md should also call out the mutate-first tradeoff": Reasonable suggestion in principle but the failure-mode discussion is genuinely a code-site concern (about how to write similar handlers), not a tutorial-narrative concern (about what the pattern does). The prose already explains the recursion-guard / no-infinite-loop story which is the what; the comment explains the trade-off you accept by adopting this pattern. Keeping that split intentional.

"Mount returns state unchanged — add a one-line comment": Defensible suggestion but the line return s, nil immediately after the Subscribe error-check is, in idiomatic Go, the canonical signal that no state changed. Adding // state unchanged; subscribe is the only effect would over-document for any reader who's read more than a few Go controllers. Holding the current shape.

The PR is in good shape; CI is green; the round-3 verifications (go.sum hash legitimacy + new line-include directives + Publish error propagation) all landed correctly. Calling convergence.

[claude]

Code Review — Phase 6: livetemplate v0.10.0 / Subscribe+Publish migration

Overview

This PR completes the BroadcastAction → Subscribe/Publish migration for the two literate examples and bumps the dependency to v0.10.0. It also hardensthe Mermaid validation CI path with better Chrome flags and a more realistic timeout. The scope is focused and the PR description is thorough.

---

validate.go — Chrome flags & timeout

Positive: The added flags (--disable-dev-shm-usage, --disable-extensions, --no-first-run) are standard headless-Chrome hardening for constrained CI environments. The motivation for each is well-documented.

Concern — timeout jump: 15s → 60s is a 4× increase. The comment explains the worst-case observed, but a 60s per-file timeout means a fully hung Chrome process (e.g., zombie after a signal) would block the command for a full minute before the context fires. Consider whether a shorter ceiling (e.g., 30s) with a retry or a separate per-navigate deadline inside the loop would be more surgical.

Minor — comment verbosity: The block comment above the opts slice explains each flag in detail. That level of context is genuinely useful here (CI debugging is notoriously hard to reproduce), but the comment is longer than the code it precedes. The --disable-dev-shm-usage explanation is valuable; the --disable-extensions / --no-first-run explanation ("shave a second or two") could be a single short line.

---

counter.go (both examples) — Mount + Subscribe/Publish

Positive: Error propagation in Mount is the right call. The explicit rationale (safe-by-default for readers who copy the pattern with a developer topic) is exactly the kind of non-obvious WHY worth preserving.

Concern — Subscribe idempotency claim: The Mount comment states:

Subscribe is idempotent per-connection, so re-Mounts (which happen on every HTTP request and WS connect) are no-ops.

This is load-bearing: if Subscribe is not idempotent in v0.10.0, repeated mounts would accumulate subscriptions and cause duplicate fan-out delivery (each Publish would trigger N actions instead of 1 after N reconnects). Please confirm this is documented or tested in livetemplate#430 / the v0.10.0 release notes. If idempotency is only best-effort or connection-scoped rather than guaranteed, the comment should say so.

Concern — mutate-first ordering: The PR description acknowledges this trade-off, and the Increment comment explains it clearly. One thing not mentioned: if Publish returns an error and the caller returns (s, err), the framework will see a non-nil error from the action handler. Does livetemplate v0.10.0 roll back the state mutation on action error, or does it commit the mutated state and also propagate the error? If the framework commits state before the error is surfaced, the local connection does advance its count while peers do not — the "transient divergence" is actually permanent until reconnect. Worth a one-liner clarification in the comment.

Minor — identical files across two examples: literate-counter-include/_app/counter.go and literate-linked-include/_app/counter.go are byte-for-byte identical after this PR. That's presumably intentional (each example is self-contained), but if they diverge in the future the duplication will be a maintenance burden. Out of scope here, just flagging.

---

index.md changes (both examples)

The line-range updates (lines="13-35" → lines="13-67") correctly track the expanded counter.go. The prose rewrite clearly explains the two-step opt-in (Subscribe in Mount + Publish in handler) in a way that's easy to follow.

---

go.mod / go.sum

Clean bump. The go.mod h1 hash for v0.10.0 differs from v0.9.0 as expected; the go.mod hash is shared, which is consistent with only the module source changing between patch releases.

---

Summary

Area	Verdict
API migration correctness	✅ Looks correct
Error propagation	✅ Good
Subscribe idempotency assumption	⚠️ Needs confirmation
Mutate-first error semantics	⚠️ Needs one-liner clarification
Chrome timeout	⚠️ Consider a tighter bound or per-navigate deadline
Comment length	ℹ️ Minor — acceptable for a literate example

The two ⚠️ items on counter.go are the most worth addressing before merge, since they affect what readers will copy from the examples. The Chrome timeout is a pragmatic CI fix and the risk is low.

🤖 Generated with Claude Code

[adnaan]

Three items from the round-4 review addressed:

Subscribe idempotency (verified, no code change). Confirmed in livetemplate v0.10.0's internal/session/registry.go:567-569:
```go
if _, already := conn.subscribedTopics[topic]; already {
return false // idempotent: already subscribed
}
```
The set-based subscriber map guarantees Subscribe(SelfTopic()) is a no-op on re-Mount, regardless of how many times Mount fires across HTTP GET → POST → WS-connect → reconnect cycles. The literate-tutorial comment is correct as written.

Mutate-first error semantics (verified, comment corrected). Verified livetemplate v0.10.0's dispatch path: DispatchWithState returns (newState, err); the framework only assigns connSt.state = newState on err == nil. On (s, err), newState is discarded. So Publish-error semantics:

• Action returns (s, err) after Publish failure
• Framework discards s; this connection stays at pre-action state
• Peers never received the failed Publish either (Publish errored before the dispatch loop fanned out)
• Both sides at pre-action state — no divergence by construction

Previous comment incorrectly claimed "transient divergence." Corrected in 74f51ad to reflect the actual contract.

Chrome timeout 15s → 60s (declining the "tighter bound + retry" suggestion). The 4× jump is deliberate. A "tight bound + retry" approach (e.g. 30s × 2 retries) would double the worst-case latency on a genuine Chrome hang (60s vs. 30s+30s=60s; same ceiling, more complexity). The current 60s is a single deadline that catches both slow cold-starts (the actual common-case failure) and pathological hangs (extremely rare; would also be caught by the outer CI test-job timeout if Chrome truly zombied). The flag-hardening (--disable-dev-shm-usage) is the root-cause fix; the timeout bump is the safety margin while the slow Ubuntu Actions cold-start is the new floor. Keeping the simpler shape.

The other round-4 nits (comment length, identical-files cross-example) are stylistic and declined — comment depth is intentional for a literate tutorial; cross-example identity is the right outcome for two intentionally-parallel examples (a future divergence would be a deliberate teaching choice, not duplication-creep).

[claude]

Code Review — Phase 6: Subscribe/Publish migration (updated review)

Note: A prior automated review comment on this PR incorrectly stated that ctx.Subscribe errors are silently discarded. The actual diff shows they are properly propagated — Mount returns (s, err) on failure. Flagging this so it doesn't create confusion.

---

What looks good

• Error handling is correct throughout. Mount propagates Subscribe errors; all three action handlers propagate Publish errors. The old BroadcastAction calls were fire-and-forget with no return value — this PR is strictly safer.
• Mutate-first ordering is correctly explained in Increment. The comment makes clear that returning (s, err) causes the dispatcher to discard newState, so a failed Publish leaves all connections at their pre-action state with no divergence. This is non-obvious behavior and worth documenting.
• Chrome CI flags are well-chosen. --disable-dev-shm-usage is the standard fix for the D-Bus/shm OOM on Docker-based runners. --disable-extensions and --no-first-run are low-risk startup accelerators.
• Both literate examples are in sync — the counter.go changes are identical across literate-counter-include and literate-linked-include, which is correct for independent copy-pasteable examples.
• go.sum is consistent — old v0.9.0 hashes removed, new v0.10.0 hashes added, the /go.mod module-file hash is correctly shared.

---

Issues / Suggestions

1. Timeout increase (15s → 60s) could mask real Chrome hangs

The 4× bump is explained and reasonable for the described CI environment. One thing to consider: the timeout wraps the entire file — cold Chrome start plus all Navigate + Sleep(2s) + Evaluate cycles for every diagram. If a file has many diagrams, 60s might still be tight; if Chrome truly hangs, you wait the full 60s per invocation before getting any signal. Not blocking, but worth a follow-up if CI flakiness resurfaces.

2. highlight line number in literate-counter-include/index.md

-```go include="./_app/counter.go" lines="13-35" highlight="20"
+```go include="./_app/counter.go" lines="13-66" highlight="45"

Line 45 of the new counter.go is the func (c *CounterController) Increment(...) signature, which is reasonable to highlight. Worth a quick visual check that the rendered output highlights the intended line — literate include tooling can behave unexpectedly if highlight is an absolute line number vs. relative to lines start.

3. Comment in Increment explains framework internals that could go stale

// Mutate-first ordering is safe even when Publish errors: the
// livetemplate dispatcher only assigns the returned newState to the
// connection's persisted state when the action returns (state, nil).

This is accurate for v0.10.0. If livetemplate's dispatch semantics change in a later version, this comment will silently lie to readers. It might be more durable to link to the livetemplate docs or a specific issue/commit rather than restating the implementation invariant inline.

4. Minor prose nit in main.go comment (both examples)

// the constant groupID here is what makes the literate-linked tutorial
// demonstrate cross-region state sharing.

"literate-linked tutorial" appears in both literate-counter-include/main.go and literate-linked-include/main.go. The reference makes sense in the linked example but is slightly off in the counter example, which is a standalone tutorial. Very minor.

---

Summary

The migration is mechanically correct, error handling is properly added, and the Chrome CI fixes are sound. The suggestions above are non-blocking. LGTM with those minor points in mind.