fix(chatboxes): keep the session flow headers with their columns - #3673
Conversation
On a wide panel the SENTIMENT header sat hundreds of pixels from the column it names. The headers were a four-cell CSS grid stretched across the panel while the diagram was a fixed-width SVG: two coordinate systems that only agreed when the panel happened to be the chart's width, so the first three columns lined up by coincidence and the fourth did not line up at all. They are drawn inside the SVG now, at the `columnX` the layout already computed. Sharing one coordinate space is the only arrangement in which a header and its column cannot drift, and the layout echoes those positions back specifically so there is one source for both. The last column also labelled to the LEFT of its bar — a defensive choice from before the gutter was reserved, which by now just put the text on top of the ribbons arriving at it. Every column labels to the right, into the space that was already there for it. Both are pinned: the layout test asserts every node sits on its column's x, and the component test asserts four distinct, increasing header positions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b9861249-3812-4830-8636-d3404036f33b) |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughThe Sankey layout now returns column x-coordinates. The diagram renders stage headers inside the SVG using those coordinates. Links and nodes move below the header band. Labels and counts render to the right of all node bars. Tests verify column alignment and ordered header positions. A patch changeset documents the update. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
mcpjam-inspector/client/src/components/chatboxes/ChatboxInsightsSankey.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/components/chatboxes/__tests__/ChatboxInsightsSankey.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/components/chatboxes/__tests__/insights-sankey.test.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/chatboxes/__tests__/insights-sankey.test.ts (1)
212-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
STAGE_ORDERas the test source of truth.The test repeats stage names and their order in two literals, while
layoutSankeyindexescolumnXbySTAGE_ORDER. If a stage is added or reordered, this test can skip coverage or assert the wrong column. Iterate overSTAGE_ORDERwith its index instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcpjam-inspector/client/src/components/chatboxes/__tests__/insights-sankey.test.ts` around lines 212 - 226, Update the “echoes the column positions back for the headers to share” test to iterate over STAGE_ORDER with each stage’s index, replacing both repeated stage-name literals. Use that index to compare nodes against laid.columnX, while preserving the empty-stage skip and existing assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@mcpjam-inspector/client/src/components/chatboxes/__tests__/ChatboxInsightsSankey.test.tsx`:
- Around line 302-318: Strengthen the test around the Sankey header-rendering
implementation by adding a stable stage marker to each header element, then
query headers by that marker rather than generic text nodes. In the test “draws
each column header at its own column's x”, map each stage header to its
corresponding node column and assert matching x coordinates, while retaining the
four-header coverage.
In `@mcpjam-inspector/client/src/components/chatboxes/ChatboxInsightsSankey.tsx`:
- Around line 48-50: Update the columnX calculation in ChatboxInsightsSankey so
columns are positioned between explicit left and right bounds, with the final
node ending exactly at VIEW_WIDTH - LABEL_GUTTER. Remove the extra left-margin
effect from the right-edge calculation while preserving the intended left margin
and existing column spacing.
---
Nitpick comments:
In
`@mcpjam-inspector/client/src/components/chatboxes/__tests__/insights-sankey.test.ts`:
- Around line 212-226: Update the “echoes the column positions back for the
headers to share” test to iterate over STAGE_ORDER with each stage’s index,
replacing both repeated stage-name literals. Use that index to compare nodes
against laid.columnX, while preserving the empty-stage skip and existing
assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 428f2b06-605a-414c-94e3-ee51d5da8770
📒 Files selected for processing (5)
.changeset/insights-flow-header-alignment.mdmcpjam-inspector/client/src/components/chatboxes/ChatboxInsightsSankey.tsxmcpjam-inspector/client/src/components/chatboxes/__tests__/ChatboxInsightsSankey.test.tsxmcpjam-inspector/client/src/components/chatboxes/__tests__/insights-sankey.test.tsmcpjam-inspector/client/src/components/chatboxes/insights-sankey.ts
| it("draws each column header at its own column's x", () => { | ||
| // Guards the misalignment that shipped: headers laid out by CSS across the | ||
| // full panel while the columns lived in a fixed-width SVG. | ||
| renderSankey(); | ||
| const headers = Array.from(document.querySelectorAll("text")).filter((t) => | ||
| ["GOAL", "BEHAVIOR", "OUTCOME", "SENTIMENT"].includes( | ||
| (t.textContent ?? "").toUpperCase(), | ||
| ), | ||
| ); | ||
| expect(headers).toHaveLength(4); | ||
| const xs = headers.map((h) => Number(h.getAttribute("x"))); | ||
| // Strictly increasing, and the last one is nowhere near the right edge — | ||
| // it sits over its column, with the label gutter beyond it. | ||
| expect(xs).toEqual([...xs].sort((a, b) => a - b)); | ||
| expect(new Set(xs).size).toBe(4); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert each header against its stage column.
This test checks only that four matching text nodes have unique, sorted x values. It can pass when all headers are shifted together or assigned to the wrong stages. querySelectorAll("text") can also include a node label with the same text as a stage title. Add a stable stage marker to header elements and compare each header’s x with the corresponding node column.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@mcpjam-inspector/client/src/components/chatboxes/__tests__/ChatboxInsightsSankey.test.tsx`
around lines 302 - 318, Strengthen the test around the Sankey header-rendering
implementation by adding a stable stage marker to each header element, then
query headers by that marker rather than generic text nodes. In the test “draws
each column header at its own column's x”, map each stage header to its
corresponding node column and assert matching x coordinates, while retaining the
four-header coverage.
| const VIEW_WIDTH = 1160; | ||
| /** Reserved to the right of the last column for its labels. */ | ||
| const LABEL_GUTTER = 260; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reserve the full LABEL_GUTTER after the last node.
The columnX calculation at Lines 111-115 adds the 40px left margin after subtracting LABEL_GUTTER. With VIEW_WIDTH = 1160 and LABEL_GUTTER = 260, the final bar ends at 940, so only 220px remains in the viewBox. The 10px label gap leaves about 210px for the label. Derive the columns between explicit left and right bounds so the final bar ends at VIEW_WIDTH - LABEL_GUTTER.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mcpjam-inspector/client/src/components/chatboxes/ChatboxInsightsSankey.tsx`
around lines 48 - 50, Update the columnX calculation in ChatboxInsightsSankey so
columns are positioned between explicit left and right bounds, with the final
node ending exactly at VIEW_WIDTH - LABEL_GUTTER. Remove the extra left-margin
effect from the right-edge calculation while preserving the intended left margin
and existing column spacing.
The SENTIMENT header sits hundreds of pixels from the column it names.
Cause
Two coordinate systems that only agreed by accident. The stage headers were a four-cell CSS
grid-cols-4stretched across the full panel; the diagram is a fixed-width SVG. On a panel wider than the chart the grid keeps stretching and the columns do not, so GOAL / BEHAVIOR / OUTCOME lined up roughly by coincidence and SENTIMENT — the one pinned to the right edge bytext-right— did not line up at all.Fix
Headers are drawn inside the SVG, at the
columnXthe layout already computes.layoutSankeynow echoes those positions back specifically so the headers and the nodes read from one source; sharing a coordinate space is the only arrangement in which they cannot drift.Also unflipped the last column's labels. They were anchored to the LEFT of their bar — a defensive choice from before the label gutter existed — which by now just painted the text on top of the ribbons arriving at that column and read as a rendering fault. Every column labels to the right, into the gutter already reserved for it. The viewBox widened to 1160 with a 260px gutter so the longest sentiment names ("Neutral, mild curiosity") fit without truncation.
Pinned
insights-sankey.test.ts— every node sits on its own column'sx, and the layout returns those positions.ChatboxInsightsSankey.test.tsx— four distinct, strictly increasing header positions.Neither test passes against the old arrangement, since the old headers were not in the SVG at all.
Verification
npm run typecheck:client— cleannpm run build:inspector— succeedsRendering still wants a real look: jsdom places the text but does not measure it, so whether the longest theme names actually clear the gutter is not something these tests can answer.
🤖 Generated with Claude Code
Note
Low Risk
Insights Sankey presentation-only changes with regression tests; no API, auth, or data handling impact.
Overview
Fixes session flow column headers drifting away from their Sankey columns on wide panels by moving stage titles into the SVG at
layout.columnXinstead of a full-width CSS grid above a fixed-width chart.layoutSankeynow returnscolumnXso headers and nodes share one coordinate system; diagram content is shifted down by a newHEADER_HEIGHTband.Sentiment (and all) node labels now anchor to the right of each bar into the label gutter, avoiding text drawn over incoming ribbons. The view widens to 1160px with a 260px gutter for long theme names. Tests lock header
xpositions and layoutcolumnXecho.Reviewed by Cursor Bugbot for commit aa2d17f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes misaligned session flow headers by rendering them inside the SVG at the computed column positions, so headers stay aligned with their columns on any panel width. Also moves the last column's labels to the right and expands the label gutter to prevent overlap.
layoutSankey’scolumnX; add a header band and translate nodes/links accordingly.layoutSankeynow returnscolumnXso nodes and headers share one coordinate space.Written for commit aa2d17f. Summary will update on new commits.