refactor(realtime): drop unused Hub.PublishMany batch wrapper - #176
refactor(realtime): drop unused Hub.PublishMany batch wrapper#176KrasimirKralev wants to merge 1 commit into
Conversation
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.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 29, 2026, 5:08 AM ET / 09:08 UTC. ClawSweeper reviewWhat this changesThe PR removes the internal realtime hub’s unused multi-event publishing helper and updates its sole test call to publish one event directly. Merge readinessKeep open: current main still contains the wrapper, while the submitted cleanup is narrowly scoped, source-supported, and has no actionable correctness finding. Priority: P3 Review scores
Verification
How this fits togetherThe 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]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest 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. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
What Problem This Solves
apps/api/internal/realtime/hub.goexportsHub.PublishMany([]store.Event), athin batch wrapper that loops over its slice and calls
Hub.Publishper event.It has no production caller. Every runtime emit site publishes a single
event through
Hub.Publish—httpapi/features.go:602,:1074,:1089,:1676, andserver.go:841— and nothing in the codebase batch-publishes. Themethod is reachable only from one line in
hub_test.go, and that call passes asingle-element slice, so it exercises nothing
Hub.Publishdoesn'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 PublishManyshows asingle 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{{…}})toPublish(store.Event{…})— asingle-element emit, so the workspace-isolation assertion it guards (an event
for
wsp_othermust not reach awsp_1subscriber) is preserved byte-for-bytein intent. Behavior-preserving, single production file, net −6 production LOC.
No new symbols, config, or dependencies.
Hub.Publishand every other Hub methodare untouched.
User Impact
None. No runtime, HTTP, WebSocket, CLI, config, or public-API behavior change —
internal/realtimeis not importable outside the module, and the removed methodwas reachable from no production path.
Evidence
Branched off current
main(e617cb5), Linux, Go 1.26.6 (matchesgo.mod),go builddeps 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 theclickclackbinary on cleanmain, and no longer reports it after the change,introducing no new unreachable funcs:
CI deadcode gate stays green — the repo's
deadcode -test ./...gatereports 0 unreachable funcs both before and after (the test-emit now goes
through
Publish), so this change does not touch that gate.No dangling reference —
go vet ./apps/api/...(compiles all packages andtheir tests) → exit 0.
Changed package green —
go test ./apps/api/internal/realtime/→ok.gofmt -lclean on both files;go build ./apps/api/...→ exit 0.Diff: −6 / +… , 2 files (
hub.go−6 production;hub_test.goone linerepointed). 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