Skip to content

refactor(realtime): drop unused Hub.PublishMany batch wrapper - #176

Draft
KrasimirKralev wants to merge 1 commit into
openclaw:mainfrom
KrasimirKralev:refactor/drop-unused-publishmany
Draft

refactor(realtime): drop unused Hub.PublishMany batch wrapper#176
KrasimirKralev wants to merge 1 commit into
openclaw:mainfrom
KrasimirKralev:refactor/drop-unused-publishmany

Conversation

@KrasimirKralev

Copy link
Copy Markdown
Contributor

What Problem This Solves

apps/api/internal/realtime/hub.go exports Hub.PublishMany([]store.Event), a
thin batch wrapper that loops over its slice and calls Hub.Publish per event.
It has no production caller. Every runtime emit site publishes a single
event through Hub.Publishhttpapi/features.go:602, :1074, :1089,
:1676, and server.go:841 — and nothing in the codebase batch-publishes. The
method is reachable only from one line in hub_test.go, and that call passes a
single-element slice, so it exercises nothing Hub.Publish doesn't already.

This is not a bug — the wrapper is correct. It is speculative surface: it was
added without a caller and has never had one (a git log -S PublishMany shows a
single introducing commit and no later use), so it is dead-from-birth batch API
with no batch-emit path to serve.

Why This Change Was Made

Debt cleanup (family: dead-code removal), same shape as the merged
#165. Remove the unused
method and repoint the one test that used it (TestHubSubscribePublishAndUnsubscribe)
from PublishMany([]store.Event{{…}}) to Publish(store.Event{…}) — a
single-element emit, so the workspace-isolation assertion it guards (an event
for wsp_other must not reach a wsp_1 subscriber) is preserved byte-for-byte
in intent. Behavior-preserving, single production file, net −6 production LOC.
No new symbols, config, or dependencies. Hub.Publish and every other Hub method
are untouched.

User Impact

None. No runtime, HTTP, WebSocket, CLI, config, or public-API behavior change —
internal/realtime is not importable outside the module, and the removed method
was reachable from no production path.

Evidence

Branched off current main (e617cb5), Linux, Go 1.26.6 (matches go.mod),
go build deps from source.

  • Dead-code proof — the same analyzer the repo's CI runs
    (golang.org/x/tools/cmd/deadcode) flags the method as unreachable from the
    clickclack binary on clean main, and no longer reports it after the change,
    introducing no new unreachable funcs:

    $ go run golang.org/x/tools/cmd/deadcode@latest ./apps/api/cmd/clickclack/   # before
    apps/api/internal/realtime/hub.go:60:15: unreachable func: Hub.PublishMany
    … (2 unrelated store helpers)
    $ # after
    … (only the 2 unrelated store helpers remain; PublishMany gone)
  • CI deadcode gate stays green — the repo's deadcode -test ./... gate
    reports 0 unreachable funcs both before and after (the test-emit now goes
    through Publish), so this change does not touch that gate.

  • No dangling referencego vet ./apps/api/... (compiles all packages and
    their tests) → exit 0.

  • Changed package greengo test ./apps/api/internal/realtime/ok.

  • gofmt -l clean on both files; go build ./apps/api/... → exit 0.

Diff: −6 / +… , 2 files (hub.go −6 production; hub_test.go one line
repointed). Same family as the merged dead-helper cleanup #165.

AI-assisted contribution. Happy to close this if the wrapper is intended as
convenience API to keep for a future batch-emit path.

Opened from an org-owned fork via the API; if GitHub's Allow edits by maintainers toggle isn't honored on this PR, a maintainer can still push to the branch or supersede-and-land.


Generated by Claude Code

Hub.PublishMany is a batch wrapper over Hub.Publish with no production
caller. deadcode reports it unreachable from the clickclack binary; every
runtime emit site calls Publish with a single event, and the method has
never had a production caller since it was introduced. Remove it and
repoint the one single-element test emit to Publish, preserving the
workspace-isolation assertion. Behavior-preserving, net -6 production LOC.
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 27, 2026
@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 29, 2026, 5:08 AM ET / 09:08 UTC.

ClawSweeper review

What this changes

The PR removes the internal realtime hub’s unused multi-event publishing helper and updates its sole test call to publish one event directly.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: current main still contains the wrapper, while the submitted cleanup is narrowly scoped, source-supported, and has no actionable correctness finding.

Priority: P3
Reviewed head: 12e4740f06c75558b8fb730b47ac50c82a9d4c02

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) This is a focused, source-supported cleanup with direct terminal validation evidence and no identified patch defect.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The changed production owner is the realtime hub’s unused PublishMany wrapper; the supplied before/after dead-code trace exercises the clickclack binary and the after-change package test confirms the direct Publish workspace-isolation path still passes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The changed production owner is the realtime hub’s unused PublishMany wrapper; the supplied before/after dead-code trace exercises the clickclack binary and the after-change package test confirms the direct Publish workspace-isolation path still passes.
Evidence reviewed 5 items Current-main call graph: Current main retains PublishMany, and its only occurrences are the method definition and the single-element test call; there is no production caller to preserve.
Introduced change: The pinned PR delta removes only the wrapper and replaces its single-element test use with Publish, retaining the workspace-isolation assertion.
Realtime contract: The architecture documentation defines this package as the in-process workspace event hub; the existing test continues to cover workspace-scoped delivery through Publish.
Findings None None.
Security None None.

How this fits together

The API’s realtime hub receives stored workspace events from server handlers, filters them by workspace, and forwards matching events to subscribed WebSocket paths. It is the in-process live-delivery layer beside durable event recovery.

flowchart LR
A[API event emitters] --> B[Realtime hub]
B --> C[Workspace filter]
C --> D[Subscribed clients]
D --> E[Live event delivery]
Loading

Before merge

  • Complete next step (P2) - No repair lane is needed because the patch has no actionable defect; it awaits ordinary maintainer review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Change size 2 files; production −6, tests 1 call changed The diff is limited to deleting the unreachable wrapper and preserving its sole test scenario.

Technical review

Best possible solution:

Remove the obsolete internal wrapper while retaining direct publishing and the existing workspace-isolation coverage.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR is a cleanup, not a report of failing user behavior; source inspection establishes the relevant call graph.

Is this the best way to solve the issue?

Yes: removing the wrapper is the narrowest maintainable cleanup because current main has no production reference and its only test use passes one event.

AGENTS.md: found, but no applicable review policy affected this item.

Codex review notes: model internal, reasoning high; reviewed against 486fd23545af.

Labels

Label justifications:

  • P3: This is a small internal dead-code cleanup with no supported user-visible behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the realtime hub’s unused PublishMany wrapper; the supplied before/after dead-code trace exercises the clickclack binary and the after-change package test confirms the direct Publish workspace-isolation path still passes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the realtime hub’s unused PublishMany wrapper; the supplied before/after dead-code trace exercises the clickclack binary and the after-change package test confirms the direct Publish workspace-isolation path still passes.

Evidence

What I checked:

  • Current-main call graph: Current main retains PublishMany, and its only occurrences are the method definition and the single-element test call; there is no production caller to preserve. (apps/api/internal/realtime/hub.go:60, 486fd23545af)
  • Introduced change: The pinned PR delta removes only the wrapper and replaces its single-element test use with Publish, retaining the workspace-isolation assertion. (apps/api/internal/realtime/hub_test.go:27, 12e4740f06c7)
  • Realtime contract: The architecture documentation defines this package as the in-process workspace event hub; the existing test continues to cover workspace-scoped delivery through Publish. (docs/architecture/overview.md:27, 12e4740f06c7)
  • Feature history: History identifies the realtime recovery work as the most recent substantive change on this path, providing a routing candidate for review. (apps/api/internal/realtime/hub.go:47, bfc8fc6230e4)
  • Submitted runtime evidence: The PR body supplies before/after dead-code analyzer output for the clickclack binary and successful after-change realtime package tests, vet, and build checks.

Likely related people:

  • Peter Steinberger: The current hub path’s latest substantive history is the realtime recovery change. (role: recent realtime feature contributor; confidence: medium; commits: bfc8fc6230e4; files: apps/api/internal/realtime/hub.go, apps/api/internal/realtime/hub_test.go)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-27T02:42:46.843Z sha 12e4740 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-28T23:01:40.156Z sha 12e4740 :: needs maintainer review before merge. :: none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant