Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,14 @@ export function CorrectionDecayCurve({
/>
{/* Visual graduation markers: dashed vertical lines mapped to
graduation timestamps. Numeric XAxis above is what makes
ReferenceLine x={ms} actually align with the curve. */}
ReferenceLine x={ms} actually align with the curve. The
custom `label` renders an invisible SVG <g> carrying the
test hook attribute + accessible <title> tooltip shown on
hover (native SVG — no extra JS). */}
{visibleMarkers.map((l) => {
const gMs = l.graduated_at ? new Date(l.graduated_at).getTime() : null
if (gMs === null) return null
const ariaLabel = `Rule graduated: ${l.description ?? l.id}`
return (
<ReferenceLine
key={`refline-${l.id}`}
Expand All @@ -117,6 +121,33 @@ export function CorrectionDecayCurve({
strokeOpacity={0.4}
strokeDasharray="4 4"
ifOverflow="extendDomain"
label={(props: { viewBox?: { x?: number; y?: number; height?: number } }) => {
const vb = props?.viewBox ?? {}
const cx = typeof vb.x === 'number' ? vb.x : 0
const yTop = typeof vb.y === 'number' ? vb.y : 0
const h = typeof vb.height === 'number' ? vb.height : 0
return (
<g
data-graduation-marker=""
data-lesson-id={l.id}
data-graduated-at={l.graduated_at ?? ''}
data-x={cx}
role="img"
aria-label={ariaLabel}
>
<title>{ariaLabel}</title>
{/* transparent hit-target for hover tooltip */}
<rect
x={cx - 6}
y={yTop}
width={12}
height={h}
fill="transparent"
pointerEvents="all"
/>
</g>
)
}}
/>
)
})}
Expand Down Expand Up @@ -161,21 +192,6 @@ export function CorrectionDecayCurve({
</ComposedChart>
</ResponsiveContainer>
</div>
{/* Hidden marker list — a11y fallback + test hook. The visible
dashed vertical lines are rendered above via <ReferenceLine>,
but Recharts emits SVG that screen readers don't surface well,
so we keep this list as the accessible representation. Tests
also count [data-graduation-marker] from this list. */}
<div aria-hidden className="hidden">
{visibleMarkers.map((l) => (
<span
key={l.id}
data-graduation-marker
data-lesson-id={l.id}
data-graduated-at={l.graduated_at ?? ''}
/>
))}
</div>
{visibleMarkers.length > 0 && (
<div className="mt-2 text-[11px] text-[var(--color-body)]">
{visibleMarkers.length} rule graduation{visibleMarkers.length === 1 ? '' : 's'} in range
Expand Down
31 changes: 31 additions & 0 deletions cloud/dashboard/tests/CorrectionDecayCurve.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,37 @@ describe('CorrectionDecayCurve graduation markers', () => {
expect(markers.length).toBe(2)
})

it('positions marker at correct x for lesson graduated 5 days ago (7d range)', () => {
const corrections = Array.from({ length: 10 }, (_, i) => mkCorr(`c${i}`, i + 1))
const lessons = [mkLesson('five-days', daysAgo(5))]
const { container } = render(
<CorrectionDecayCurve corrections={corrections} lessons={lessons} range="7d" />,
)
const marker = container.querySelector('[data-graduation-marker][data-lesson-id="five-days"]')
expect(marker).not.toBeNull()
// 5 days ago in a 7d range is ~2/7 of the way from left; with the mocked
// 800px wide container and Recharts' default Y-axis width, the x pixel
// should land in the left third of the plot area (i.e. < 400, > 0).
const x = Number(marker!.getAttribute('data-x'))
expect(Number.isFinite(x)).toBe(true)
expect(x).toBeGreaterThan(0)
expect(x).toBeLessThan(400)
})

it('marker carries accessible aria-label + SVG <title> for tooltip', () => {
const corrections = [mkCorr('c0', 1)]
const lessons: Lesson[] = [
{ ...mkLesson('a', daysAgo(3)), description: 'Avoid em dashes' } as Lesson,
]
const { container } = render(
<CorrectionDecayCurve corrections={corrections} lessons={lessons} range="7d" />,
)
const marker = container.querySelector('[data-graduation-marker][data-lesson-id="a"]')
expect(marker).not.toBeNull()
expect(marker!.getAttribute('aria-label')).toMatch(/Avoid em dashes/)
expect(marker!.querySelector('title')?.textContent).toMatch(/Avoid em dashes/)
})

it('caps markers at 12 and renders "+N more" note', () => {
const corrections = Array.from({ length: 30 }, (_, i) => mkCorr(`c${i}`, i + 1))
const lessons = Array.from({ length: 15 }, (_, i) => mkLesson(`r${i}`, daysAgo(i + 1)))
Expand Down
6 changes: 4 additions & 2 deletions cloud/dashboard/tests/dashboard-page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ describe('/dashboard preview-with-sample-data flow', () => {
// Fixture-backed panels render
expect(screen.getByText('Time Saved')).toBeInTheDocument()
expect(screen.getByText('Your Rules')).toBeInTheDocument()
// Demo lessons appear (from demo-dashboard fixture)
expect(screen.getByText(/Never use em dashes/i)).toBeInTheDocument()
// Demo lessons appear (from demo-dashboard fixture). Use getAllByText
// because a graduated lesson's description also surfaces in the decay
// curve's SVG <title> tooltip on its graduation marker.
expect(screen.getAllByText(/Never use em dashes/i).length).toBeGreaterThan(0)

// Exit demo
await user.click(screen.getByRole('button', { name: /Exit demo/i }))
Expand Down
23 changes: 23 additions & 0 deletions cloud/dashboard/tests/setup.ts
Original file line number Diff line number Diff line change
@@ -1 +1,24 @@
import '@testing-library/jest-dom'
import { vi } from 'vitest'
import * as React from 'react'

// Recharts' ResponsiveContainer uses getBoundingClientRect which returns 0 in
// jsdom, collapsing charts to width/height -1 and skipping render. Mock it with
// a fixed-size wrapper so Recharts actually renders SVG (required for tests
// that assert on chart-level custom SVG attributes like [data-graduation-marker]).
vi.mock('recharts', async (importOriginal) => {
const actual = await importOriginal<typeof import('recharts')>()
return {
...actual,
ResponsiveContainer: ({ children }: { children: React.ReactNode }) =>
React.createElement(
'div',
{ style: { width: 800, height: 400 } },
React.createElement(
actual.ResponsiveContainer,
{ width: 800, height: 400 },
children,
),
Comment on lines +13 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether any ResponsiveContainer call sites rely on props beyond width/height.
rg -nP --type=tsx -C2 '<ResponsiveContainer\b'

Repository: Gradata/gradata

Length of output: 86


🏁 Script executed:

# First, verify the file exists and check its structure
ls -la cloud/dashboard/tests/setup.ts

Repository: Gradata/gradata

Length of output: 137


🏁 Script executed:

# Search for ResponsiveContainer usages without the tsx type flag
rg -nP -C2 '<ResponsiveContainer\b' --max-count=20

Repository: Gradata/gradata

Length of output: 785


🏁 Script executed:

# Also examine the actual code at the specified lines in setup.ts
head -30 cloud/dashboard/tests/setup.ts | cat -n

Repository: Gradata/gradata

Length of output: 1100


🏁 Script executed:

# Search more broadly for all ResponsiveContainer usages to see what props are passed
rg 'ResponsiveContainer' --max-count=30 -A3

Repository: Gradata/gradata

Length of output: 2613


🏁 Script executed:

# Check the recharts type definitions or documentation to understand what props ResponsiveContainer accepts
# First, check if there's a node_modules or package.json
ls cloud/dashboard/package.json 2>/dev/null || ls package.json 2>/dev/null

Repository: Gradata/gradata

Length of output: 87


🏁 Script executed:

# Look for recharts imports to understand the library version
rg 'from ["\']recharts' --max-count=10

Repository: Gradata/gradata

Length of output: 127


🏁 Script executed:

# Check recharts version in package.json
cat cloud/dashboard/package.json | grep -A2 -B2 recharts

Repository: Gradata/gradata

Length of output: 192


🏁 Script executed:

# Search for any other ResponsiveContainer prop patterns or attributes
rg 'ResponsiveContainer' -B1 -A5 cloud/dashboard/src

Repository: Gradata/gradata

Length of output: 2473


Consider forwarding extra props through the ResponsiveContainer mock for better completeness.

The mock currently accepts only children and drops all other props passed to ResponsiveContainer. While the single visible usage only passes width and height (which the mock overrides anyway), forwarding additional props makes the mock more robust and future-proof in case other props are needed.

The proposed fix is sound:

♻️ Proposed fix
-    ResponsiveContainer: ({ children }: { children: React.ReactNode }) =>
+    ResponsiveContainer: (
+      { children, ...props }: React.ComponentProps<typeof actual.ResponsiveContainer>,
+    ) =>
       React.createElement(
         'div',
         { style: { width: 800, height: 400 } },
         React.createElement(
           actual.ResponsiveContainer,
-          { width: 800, height: 400 },
+          { ...props, width: 800, height: 400 },
           children,
         ),
       ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ResponsiveContainer: ({ children }: { children: React.ReactNode }) =>
React.createElement(
'div',
{ style: { width: 800, height: 400 } },
React.createElement(
actual.ResponsiveContainer,
{ width: 800, height: 400 },
children,
),
ResponsiveContainer: (
{ children, ...props }: React.ComponentProps<typeof actual.ResponsiveContainer>,
) =>
React.createElement(
'div',
{ style: { width: 800, height: 400 } },
React.createElement(
actual.ResponsiveContainer,
{ ...props, width: 800, height: 400 },
children,
),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cloud/dashboard/tests/setup.ts` around lines 13 - 21, The ResponsiveContainer
mock currently only accepts children and drops any other props; update the mock
signature for ResponsiveContainer to accept and forward all props (e.g., ({
children, ...props })) and spread those props into both the outer div and the
call to actual.ResponsiveContainer
(React.createElement(actual.ResponsiveContainer, { width: 800, height: 400,
...props }, children)) so additional props are preserved while still keeping the
fixed size used in the test.

),
}
})
Loading