feat(web): pixel drop-shadow follows node variant + cycles per multi-data pixel#60
Merged
Merged
Conversation
…data pixel
Two related fixes for animated pixel ("carrot") shadows:
1. Same-sprite nodes now have matching pixel shadows.
DataPixel resolved its drop-shadow color from a hard-coded NODE_COLORS
map keyed on raw node type, so two nodes that fall back to the same
orange sprite (e.g. an `endpoint` and a `service`) emitted shadows in
different colors (blue vs grey) even though their visible sprite hue
was identical.
Compute per-node variant assignments once on the topology
(`topology.nodeVariants`) using the shared `lib/pixel-palette`. Both
the sprite filter (in flow-layout) and the pixel drop-shadow (in
FlowCanvas's nodeTypeMap) read from the same map so they stay in
lockstep. Nodes that happen to share the orange sprite share an
orange drop-shadow, regardless of declared type.
Threading the variants through the topology (rather than recomputing
per call site) avoids a subtle bug where flow.flow.nodes order and
topology.orderedIds order disagreed and gave a node different
palette indices in different files.
2. Multi-data steps cycle the palette per pixel.
When `step.data` is an array (multi-payload on one edge), we already
staggered the pixels by 120 ms. Each pixel now also gets a distinct
shadow hue from VARIANT_ACCENT by index (multiDataPixelColor(i)),
unless the data entry has its own `color` set (which always wins).
DataPixel gains a `pixelColor` prop (resolution: explicit pixelColor >
sourceNodeColor variant > legacy NODE_COLORS fallback).
Adds unit tests for the variant assignment, palette cycling, and a
topology-level assertion that two first-pool nodes (api endpoint,
order-service service) share the same accent color so the user sees
one consistent shadow across visually-orange sprites.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughComputes deterministic per-node sprite variants via a new palette, surfaces per-pixel color/filter props, and rewires FlowCanvas/FlowTopology to use topology-derived node variants; tests added for palette and topology behaviors. ChangesPalette, Topology, Rendering Wiring
Sequence DiagramsequenceDiagram
participant FlowCanvas
participant FlowTopologyBuilder as buildFlowTopology
participant PixelPalette as pixel-palette
participant Renderer as DataPixel
FlowCanvas->>FlowTopologyBuilder: buildFlowTopology(flow)
FlowTopologyBuilder->>PixelPalette: assignNodeVariants(orderedIds)
PixelPalette-->>FlowTopologyBuilder: nodeVariants (Map<id, NodeVariant>)
FlowTopologyBuilder-->>FlowCanvas: FlowTopology{..., nodeVariants}
FlowCanvas->>Renderer: render node sprites with variantColor/variantFilter
alt multi-data pixel
FlowCanvas->>PixelPalette: stepPixelColor(index) / stepPixelFilter(index)
PixelPalette-->>FlowCanvas: color / filter
FlowCanvas->>Renderer: render DataPixel(pixelColor, pixelFilter, delayMs)
else single-data pixel with explicit color
FlowCanvas->>Renderer: render DataPixel(pixelColor)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/FlowCanvas.tsx`:
- Around line 235-247: nodeTypeMap only iterates flow.flow.nodes so nodes added
only in the topology (e.g., from create steps) are omitted; change the logic in
the useMemo that builds nodeTypeMap (which calls buildFlowTopology) to also
include nodes present in topology.nodeVariants (or the topology's node
collection) by iterating topology.nodeVariants.keys() (or topology.nodes) and
merging them into the Map so each topology-only node gets the correct type and
variant color used later for pixel shadows.
In `@packages/web/src/lib/flow-layout.ts`:
- Line 5: Restore a file-local exported variant-cycle contract in flow-layout.ts
by adding an exported constant named VARIANT_CYCLE (and ensure VARIANT_ACCENT
remains adjacent) that aliases or filters the imported cycle from
assignNodeVariants/pixel-palette so the file still declares the 6-slot cycle
contract; update or re-export VARIANT_CYCLE here (and keep VARIANT_ACCENT in
this file) so callers and the repo rule see the local symbols while the actual
source can remain in pixel-palette, and ensure both VARIANT_CYCLE and
VARIANT_ACCENT are kept in sync when adding new variants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e96189ba-5070-494c-b61d-b3dfb3d92169
📒 Files selected for processing (6)
packages/web/__tests__/flow-layout.test.tspackages/web/__tests__/pixel-palette.test.tspackages/web/src/components/DataPixel.tsxpackages/web/src/components/FlowCanvas.tsxpackages/web/src/lib/flow-layout.tspackages/web/src/lib/pixel-palette.ts
…isibly distinct The previous attempt cycled the drop-shadow color per multi-data pixel but left the carrot sprite itself fixed at its original orange. The shadow alone was too subtle — three carrots traveling together looked like three orange dots with the same glow. Add a `multiDataPixelFilter(index)` helper that returns the same hue-rotate cycle used for the node sprite filter, and a `pixelFilter` prop on DataPixel that stacks with the drop-shadow. Multi-data steps now pass both pixelColor (drop-shadow) and pixelFilter (sprite tint) keyed by data index, so each carrot looks distinctly orange/purple/ green/blue/etc. An explicit `data[i].color` still wins on color and skips the sprite filter (the user picked the color, don't tint over it). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…f recolors Stacking hue-rotate with drop-shadow on the wrapper div didn't visibly change the carrot in the user's testing — the rotated colors and the shadow glow blended into a single orange-looking pixel. Apply the hue-rotate directly to the <img> element so the SVG sprite's own pixel colors shift, and keep drop-shadow on the wrapper so it reads from the rotated alpha and matches the glow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
120ms staggered ~7% along the path; on the self-loop ear (~100px) the three 28px carrots overlapped into a single blob and the cycled hues were not perceptible. Bump the stagger to 280ms so successive carrots stay visually separated even on short paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/web/src/components/FlowCanvas.tsx (1)
238-247:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude topology-only nodes when building
nodeTypeMap.Line 238 still loops only
flow.flow.nodes, so nodes that exist only in topology (e.g., fromcreatesteps) can miss variant/type metadata and fall back to default coloring, breaking sprite-shadow parity for those nodes.Suggested fix
const nodeTypeMap = useMemo(() => { const topology = buildFlowTopology(flow) const map = new Map<string, { type: string; color?: string }>() - for (const n of flow.flow.nodes) { - // A `custom`-typed node with an explicit hex color keeps that color - // (it already drives the sprite tint via FlowNode). Other nodes use - // the variant accent color from the shared palette. - const explicit = n.type === 'custom' && n.color ? n.color : undefined - map.set(n.id, { - type: n.type ?? 'service', - color: explicit ?? topology.nodeVariants.get(n.id)?.color, - }) + for (const id of topology.orderedIds) { + const snapshot = topology.nodeSnapshots.get(id) + const type = snapshot?.nodeType ?? 'service' + const explicit = type === 'custom' && snapshot?.color ? snapshot.color : undefined + map.set(id, { + type, + color: explicit ?? topology.nodeVariants.get(id)?.color, + }) } return map }, [flow])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/FlowCanvas.tsx` around lines 238 - 247, The node-type map building loop only iterates flow.flow.nodes so nodes present only in topology are omitted; update the code that populates the `map` (the nodeTypeMap) to iterate the union of `flow.flow.nodes` and `flow.topology?.nodes` (or `flow.topology.nodes`) and deduplicate by `n.id` (e.g., use a Set of seen ids) before inserting into `map`; keep the same logic for `explicit` color, defaulting to `topology.nodeVariants.get(n.id)?.color` and `type: n.type ?? 'service'` so topology-only nodes receive the correct variant/type metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/web/src/components/FlowCanvas.tsx`:
- Around line 238-247: The node-type map building loop only iterates
flow.flow.nodes so nodes present only in topology are omitted; update the code
that populates the `map` (the nodeTypeMap) to iterate the union of
`flow.flow.nodes` and `flow.topology?.nodes` (or `flow.topology.nodes`) and
deduplicate by `n.id` (e.g., use a Set of seen ids) before inserting into `map`;
keep the same logic for `explicit` color, defaulting to
`topology.nodeVariants.get(n.id)?.color` and `type: n.type ?? 'service'` so
topology-only nodes receive the correct variant/type metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1110f760-f6e9-4407-8096-28a33a907a25
📒 Files selected for processing (1)
packages/web/src/components/FlowCanvas.tsx
…roadcast + parallel) and apply to manual click-to-fire
Two gaps:
1. Manual click-to-fire never expanded multi-data steps. Clicking on a
node fired exactly one DataPixel per outgoing edge, regardless of
whether the step had `data: [...]`. The user saw a single carrot
with the source node's variant color, not the cycled multi-data
palette they get during auto-play.
handleNodeClick now expands every multi-data step into one
fireManualPixel call per data entry, with delayMs stagger and
pixelColor/pixelFilter pulled from the variant palette.
2. Cycling was scoped to multi-data only. A broadcast step (one source
to many targets) or a parallel step that also has multi-data still
emitted same-color carrots.
Both auto-render and manual-fire now count the total number of
carrots in the step (sum of multi-data lengths across edge flows)
and, when count >= 2, cycle the variant palette across the WHOLE
set — so a broadcast `to: [a, b, c]` produces three differently
colored carrots, and a parallel-of-broadcasts cycles across all
sub-pixels too. A 1-pixel step keeps the source node's variant
color (no cycling).
Renames the helpers from multiDataPixel{Color,Filter} to
stepPixel{Color,Filter} since they now apply to any step that emits
multiple pixels. Old names are kept as aliases for source compat.
ManualPixel grows pixelColor / pixelFilter / dataOverride / delayMs
fields, plumbed through to the rendered DataPixel so click-fires get
the same visual treatment as autoplay.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The palette now cycles through 5 colors — orange, purple, green, blue, red — for both per-node sprite hues and per-step pixel hues. Yellow was rarely surfaced (only with 6+ same-sprite nodes or 6+ carrots in one step) and didn't read well on the dark canvas. Wraps at index 5 instead of 6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Behavior is unchanged. Cleanup: - Drop the legacy NODE_COLORS map in DataPixel. sourceNodeColor is now always the variant color from the topology, so the type-keyed fallback was dead. Default to the palette's orange when nothing's set. - Drop the unused sourceNodeType prop from DataPixel and from ManualPixel (only the legacy fallback consumed it). - Remove the multiDataPixelColor / multiDataPixelFilter aliases — callers all use the stepPixel* names now. - Move the cycling decision (count carrots, decide whether to cycle, resolve per-pixel color/filter respecting explicit data.color) into resolvePixelStyle() in pixel-palette.ts; add a planStepPixels helper in FlowCanvas that walks edge flows and yields a per-pixel plan; both auto-render and manual-fire now consume planStepPixels (was ~80 lines of duplicated branching across the two code paths). Adds resolvePixelStyle unit tests covering cycle on/off and explicit data.color suppression of the sprite filter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nodeTypeMap iterated only flow.flow.nodes, so a node spawned by a `create:` step never made it in. Pixels emitted from that dynamic node fell through to the default color instead of the variant assigned to it by the topology. Iterate topology.nodeSnapshots (which already covers create-defined nodes) and look up the variant color from topology.nodeVariants. Addresses CodeRabbit review on PR #60. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes for animated pixel ("carrot") drop-shadows:
1. Same-sprite nodes now have matching pixel drop-shadows
`DataPixel` resolved its color from a hard-coded `NODE_COLORS` map keyed on raw node type, so two nodes that share a sprite hue (e.g. `api` typed `endpoint` and `worker` typed `service` — both render with the orange sprite) emitted shadows in different colors (blue vs grey).
Now: per-node variant assignment is computed once on the topology (`topology.nodeVariants`) via the shared `lib/pixel-palette` module. The sprite filter (in `flow-layout.ts`) and the pixel drop-shadow (in `FlowCanvas`'s `nodeTypeMap`) read from the same map, so they stay in lockstep. Same orange sprite → same orange drop-shadow.
Threading variants through the topology (instead of recomputing per call site) also fixes a subtle ordering bug where `flow.flow.nodes` order and `topology.orderedIds` could disagree and assign a node different palette indices in different files.
2. Multi-data steps cycle the palette per pixel
When `step.data` is an array, multiple pixels travel on one edge with a 120 ms stagger but previously shared a single color. Each pixel now gets a distinct hue from `VARIANT_ACCENT` by index (`multiDataPixelColor(i)`). Explicit `data[i].color` always wins.
Color resolution order on `DataPixel`
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests