Feat: show which events broadcast, and onto which channels - #132
Conversation
Benchmarkbase What the scan detectsDeterministic: both arms scan the same generated application on the same PHP version, and every repetition is checked to produce the same figures. A difference here is a change in what Brain detects, not machine noise — which is worth looking at rather than automatically worth fixing, since detecting more legitimately moves these.
No count changed on either corpus, including the per-node-type breakdown. TimingMedian wall clock on a shared CI runner. Noise is how far the base arm's own repetitions sat from its median, discounting the single worst on each side, with their full spread beside it; a delta inside the noise is not a measurable effect.
Phase split — Full scan — 1,188 filesA phase carries at least as much runner noise as the scan it is part of, so each one gets its own noise floor from the base arm's repetitions of that phase. Useful for locating a large move; a phase that is a few percent of the scan cannot be read at a few percent of accuracy.
Scans a synthetic Laravel application generated by |
SanderMuller
left a comment
There was a problem hiding this comment.
Reviewed at 1415293. composer test in a fresh worktree: 421 passed (1169 assertions), PHPStan No errors, Pint clean, 15/15 CI. Your number exactly.
The screenshot is in the body, so nothing to ask for there.
The gate matched the prediction on all three applications
I counted the broadcasting events in each corpus first, so the gate result would be a prediction rather than a rationalisation of whatever came out. Null arm first — base against base, identical.
events implementing ShouldBroadcast |
event nodes added | event-to-channel edges |
channel tabs changed | |
|---|---|---|---|---|
| A | 1 | 1 | 0 (no channels.php) |
0 |
| B | 2 | 2 | 0 (no channels.php) |
0 |
| C | 3 | 3 | 3 | 3 |
Nothing else moved anywhere: no node changed in place, no edge lost, no tab added or removed, and node and edge counts elsewhere are untouched. Each of C's three channel tabs went from one node and no edges to two nodes and one edge — the channel, and the event that broadcasts on it. That is the "the tab could not answer its own question" fix, measured rather than described.
And the matching claim is proven on production code, not only on the fixture. One of C's events names its channel <name>.{submissionId} while the route declares <name>.{submission_record} — different placeholder spellings, same shape, matched. That is precisely the pair a string comparison would report as two channels, and it turned up on a real application without my having to construct it.
A and B are the other half of the design working: no channels.php at all, so every channel comes out declared: false with no edge drawn, reported and not judged. On A the event implements ShouldBroadcastNow and comes out queued: false with its broadcastAs() alias attached.
The two refusals, probed
Six events against five declared channels, with a control that must match so an all-refusing probe would be visibly broken:
| event names | declared | result |
|---|---|---|
orders.{id} |
orders.{orderId} |
matched ← the control |
{team}.updates |
orders.updates |
refused — a placeholder does not swallow a literal |
{scope}.{ref} |
{tenant}.{stream} |
refused, computed: true — the shape fits exactly and it is still a coincidence |
orders.{id}.extra |
orders.{orderId} |
refused on segment count |
queues.x |
nothing | no edge, flagged undeclared |
The second refusal is the one I would have got wrong, and your corrected definition of "computed" — every segment a placeholder, rather than no literal surviving — is what makes it hold. The separator really does say nothing.
Mutations
Six, whole suite each, anchors asserted in Python and the diff printed beside the result, restores verified. Your four plus the splitter one all bite:
| mutation | result |
|---|---|
| a placeholder may swallow a literal | 1 failed |
| a computed channel is wired up anyway | 1 failed |
ShouldBroadcastNow read as queued |
2 failed |
| a plain event reported as broadcasting | 1 failed |
| the splitter no longer adds broadcasters to a channel tab | 2 failed |
Recording the first version of those four surviving, and the two defects of your own that fixing them turned up, is the most useful paragraph in the description. It is also unusual — most people delete that history.
Cost
The benchmark comment at this head: +0.8% (±0.7%) on the large scan, within noise on the small one and on method tracing, 0 counts changed. The generated corpus contains no ShouldBroadcast, no broadcastOn, no channel classes and no routes/channels.php, so that 0.8% is the idle cost — what an application pays to be told it broadcasts nothing. Gated by broadcasting.enabled on the analyzer's construction rather than on its output, which is the right place. Nothing to ask for.
Five things, none blocking
1. An empty broadcasting.paths reinstates the default behind the application's back. Measured on one fixture, three settings:
default 6 events carry broadcast data
'enabled' => false 0
'paths' => [] 6 ← app/Events, reinstated
'paths' => ['app/*/Events', …] 6
BroadcastAnalyzer::__construct does $paths !== [] ? $paths : ['app/Events']. You argued the opposite case on #129, and the argument transfers word for word: source and view paths are load-bearing so falling back there is a safety net, but "this kind is additive: an application that does not use the pattern says so with an empty array, and gets no action-class nodes rather than the default reinstated behind its back." enabled => false exists and works, so nothing is broken — the two branches just answer the same question differently.
2. Two declared routes of the same shape, and the first one silently wins. declaredChannelNode() returns on the first match. Given rooms.{roomId} and rooms.{uuid} both declared, an event naming rooms.{id} draws exactly one edge, to whichever came first, with nothing saying the choice was arbitrary. It fires zero times across the three applications I gate on — every declared route there differs in its literal segments — so this is a note, not a request. Flagging the ambiguity would fit the philosophy the rest of the pass is built on: you refuse to guess a computed name, and this is the same kind of guess.
3. broadcastQueue() with more than one return reports the first as the queue. literalReturnOf() takes the first String_ it finds:
public function broadcastQueue() { if ($this->urgent) { return 'high'; } return 'low'; }comes out queue: 'high'. Same shape as the note above: a value that cannot be known is reported as though it were known. Returning null when a method has more than one literal return would say the true thing.
4. The segment-count guard is a bounds check, and nothing is watching it. I mutated it and the suite stayed green, so I ran the behaviour both ways rather than writing it up:
Fatal error: Uncaught TypeError: str_starts_with(): Argument #1 ($haystack) must be of type string, null given
in GraphBuilder::channelNamesMatch('orders.{id}.extra', 'orders.{orderId}')
count($a) !== count($b) is not the optimisation it reads as — it is what keeps $b[$i] in bounds. Removing it crashes on any event channel with more segments than a declared one, and 421 tests pass because no fixture has that shape. Nothing is wrong with the code as written; it is one fixture line away from being pinned, and worth pinning precisely because the guard looks skippable.
5. Two comments were separated from what they document. Both are the same slip — an insertion landing after a comment instead of before it:
config/laravel-brain.php: theModel Observersheading and its whole explanation now sit above the Broadcasting block, and'observers' => [follows the broadcasting array with nothing above it. Every other section in that file is heading, comment, array.frontend/src/types/graph.ts:/** Shape of node.data.erd for model nodes… */now sits aboveBroadcastChannelData, andErdModelDatahas lost its doc comment.
Checked and fine
The Sidebar panel, rendered rather than read. An event node carrying broadcast data shows delivery: immediately, the alias, the queue, broadcastWhen() decides, broadcastWith(), not the public properties, and one row per channel with the private one named, the undeclared one carrying "no channel route here names it", and the computed one reading "name decided at runtime". An event node without the data shows no section at all, and the raw broadcast blob does not leak into the generic property list — the key !== 'broadcast' filter holds.
The bundle reproduces: npm ci && npm run build leaves git status empty, one entry chunk in and one out, no orphan, and the stylesheet is untouched because the panel reuses the existing prop-row.
edgeVisible keys on the node types at each end, and both event and channel are already registered, so the new edge type needs nothing added to the frontend registries — the #129 problem does not recur here.
Verdict
Approving. The gate landed on the predicted counts on three applications, the shape matching proved itself on a real pair of differently-spelled placeholders, five of six mutations bite, the refusals refuse, the cost is idle and gated, and the channel tab now answers the question it exists for.
62626f2 to
35b3e65
Compare
|
Conflicts @webard |
33fc7cb to
4f33558
Compare
The graph held both ends of this and nothing in between: channels were read
from routes/channels.php and events were nodes, but nothing said which event
reaches which channel -- the only question anyone asks about broadcasting.
Read from the class, not from a call chain: an event advertises itself with
ShouldBroadcast and names its channels in broadcastOn(), so coverage does not
depend on the tracer reaching it. Also carried: queued vs immediate, the alias
subscribers listen for, a literal broadcast queue, and whether broadcastWith()
or broadcastWhen() is declared.
Channel names are matched by shape, not by string: orders.{id} from the event
and orders.{orderId} from the route are one channel. A placeholder never
matches a literal, and a name whose every segment came from a value is
reported as decided at runtime rather than married to whichever declared
channel happens to fit.
Channel tabs grow forward from the channel, and these edges point at it, so
the broadcasting events are added to those tabs node by node -- seeding a walk
from them would grow each event's whole subtree into a tab about a channel.
Measured on a 60-module application: 6 broadcasting events, 6 private
channels, all six matched to a declared channel route.
4f33558 to
cedb9dc
Compare
The graph held both ends of broadcasting and nothing in between.
ChannelAnalyzerreads the channels an application authorises inroutes/channels.php, and events are nodes in their own right — but nothing said which event reaches which channel, which is the only question anyone asks about broadcasting.Read from the class, not from a call chain
An event advertises itself: it implements
ShouldBroadcastand names its channels inbroadcastOn(). So this pass is declaration-based, and its coverage does not depend on the call-chain tracer reaching the event.That is deliberate, and it is the reason to build this one before the others on the list. Every "attach facts to a node the tracer found" pass inherits the tracer's reach, and measured on a 60-module application that ceiling is real: 45 of 211 events, 27 of 113 jobs, 7 nodes against 50 files containing HTTP calls. An event nobody dispatches from a traced path still broadcasts, and still shows.
Alongside the channels the node carries what the event promises: queued or immediate (
ShouldBroadcastagainstShouldBroadcastNow), the alias subscribers listen for (broadcastAs()), a literal broadcast queue, and whetherbroadcastWith()orbroadcastWhen()is declared — a custom payload and a condition both change what a reader should expect, and neither is visible from the event's properties.Channels are matched by shape, and refused when they cannot be known
new PrivateChannel('orders.'.$this->order->id)namesorders.{id};routes/channels.phpnamesorders.{orderId}. Those are one channel, and a string comparison says they are two. Segments must agree one for one — literal against literal, placeholder against placeholder.Two refusals are the point of the rule rather than gaps in it:
{team}.updatesandorders.updatesare the same shape and the same length; letting the first segment match anything would report an event as broadcasting on a channel it may never touch.{scope}.{ref}fits{tenant}.{stream}, and that is a coincidence, not evidence.A channel with no declared counterpart still shows on the event, flagged. It is reported and not judged: an application can authorise a channel somewhere this pass does not read, so the honest line is no channel route here names it, never this is unauthorised.
One thing the edge alone did not fix
The edges were built, correct, and in no tab anyone opens. A channel tab is grown forward from the channel, and these edges point at it — so the one question a channel tab exists to answer would have been the one thing missing from it.
The broadcasting events are added to those tabs node by node rather than seeded. Seeding a forward walk from them would grow each event's entire downstream subtree into a tab that is about a channel; a test pins that the announcements tab holds exactly one channel and one event and nothing else rides in.
Verification
pintclean,phpstanno errors, 421 tests passing.Measured end to end on a 60-module application: 6 broadcasting events, 6 private channels, all six matched to a declared channel route, 6 edges.
Four mutations, all killed — a placeholder swallowing a literal, a computed channel wired up anyway,
ShouldBroadcastNowread as queued, and a plain event reported as broadcasting — plus one on the splitter change.Worth recording, since it is the reason the fixture looks the way it does: the first version of all four mutations survived, and fixing that turned up two defects of my own rather than test gaps. One rule was dead code — a second branch recomputing what the line above it had already decided. The other was my definition of "computed": I had it as no literal text survived, which is wrong, because
$scope.'.'.$refcontains a literal — the separator — that says nothing about which channel is meant. It is now every segment is a placeholder, which is both exact and the same unit the matching uses.A config switch (
laravel-brain.broadcasting.enabled, default on) gates the analyzer's construction rather than its output, andbroadcasting.pathsnames the directories — globs expanded, so a modular monolith points it at a wildcard segment.