feat(dashboard): real visual graduation markers on decay curve#47
feat(dashboard): real visual graduation markers on decay curve#47
Conversation
…urrence_at, graduated_at, correction_count
…k/PrivacyPosturePanel from primary view
Switches XAxis to type=number/scale=time using the existing ts field on DecayPoint, then renders a Recharts ReferenceLine per graduated lesson so dashed vertical markers actually overlay the curve instead of living only in a hidden a11y span list. The hidden span list is preserved as screen-reader fallback and as the test hook ([data-graduation-marker] count). Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughUpdated CorrectionDecayCurve to use a numeric timestamp X-axis (time scale) with formatted tooltip labels and visible dashed graduation markers rendered as Recharts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying gradata-dashboard with
|
| Latest commit: |
482ba9b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://459be2e6.gradata-dashboard.pages.dev |
| Branch Preview URL: | https://feat-decay-curve-visual-mark.gradata-dashboard.pages.dev |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx`:
- Around line 162-167: The accessible fallback block in CorrectionDecayCurve.tsx
is hidden from assistive tech and renders empty spans, so remove the aria-hidden
and hidden attributes from the wrapper div (or replace them with a
visually-hidden class that keeps content available to AT) and populate each span
with meaningful text (or an aria-label) describing the graduation marker (e.g.,
"graduation at X% / time Y") so screen readers can announce them; keep the
data-graduation-marker attributes and optionally add role="list" on the
container and role="listitem" on each span to improve semantics while leaving
visual rendering to the existing <ReferenceLine> SVG.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dcd983ee-abbf-46a0-8647-0f11d096c3c3
📒 Files selected for processing (1)
cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx (2)
73-79: Timestamp axis + tooltip label alignment looks correct.Using
dataKey="ts"with a numeric time scale and matching tooltip date formatting is a solid fix for coordinate/label consistency.Also applies to: 100-102
104-120: Visual graduation overlays are implemented cleanly.The dashed
ReferenceLinemapping is straightforward, keyed correctly, and consistent with the new time-based X axis.
| {/* 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"> |
There was a problem hiding this comment.
The claimed screen-reader fallback is non-functional.
Line 167 hides the block from assistive tech (aria-hidden + hidden), and Lines 169-174 render empty spans (no spoken text). If this is meant to be an a11y fallback, it currently provides no accessible output.
Proposed fix
- {/* 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>
+ {/* Screen-reader fallback + test hook for graduation markers. */}
+ <div className="sr-only">
+ <p>Rule graduations in selected range:</p>
+ <ul>
+ {visibleMarkers.map((l) => (
+ <li
+ key={l.id}
+ data-graduation-marker
+ data-lesson-id={l.id}
+ data-graduated-at={l.graduated_at ?? ''}
+ >
+ {`Lesson ${l.id} graduated on ${new Date(l.graduated_at ?? '').toLocaleDateString(
+ 'en-US',
+ { month: 'short', day: 'numeric', year: 'numeric' },
+ )}`}
+ </li>
+ ))}
+ </ul>
+ </div>Also applies to: 169-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx` around lines
162 - 167, The accessible fallback block in CorrectionDecayCurve.tsx is hidden
from assistive tech and renders empty spans, so remove the aria-hidden and
hidden attributes from the wrapper div (or replace them with a visually-hidden
class that keeps content available to AT) and populate each span with meaningful
text (or an aria-label) describing the graduation marker (e.g., "graduation at
X% / time Y") so screen readers can announce them; keep the
data-graduation-marker attributes and optionally add role="list" on the
container and role="listitem" on each span to improve semantics while leaving
visual rendering to the existing <ReferenceLine> SVG.
…umbers The prior 70%/93%/65% numbers predated the Min-2022 random-label control and weren't traceable to a validated run. Replace with the cross-model preference-adherence lift (+2.7-5.7%) and the random-label control result (3-10% regression when content is randomized) from ablation v4. Judge: blind Haiku 4.5, 432 trials, 4 models (Sonnet, DeepSeek, qwen14b, gemma4), 6 conditions. Source: project_min2022_control_passed.md, project_meta_rule_ablation_v3.md.
…learning implication Remove em dash per Oliver voice rules. Replace vague 'evidence compounds' with 'evidence accumulates' plus a concrete reference to the survival-penalty loss, so the claim is auditable instead of handwavy.
…shes Ablation v3 finding: LLM-synthesized meta-rules add value on smaller models (gemma4, qwen14b) and sit neutral on larger (Sonnet, DeepSeek). Deterministic-template meta-rules regressed and are gated out via INJECTABLE_META_SOURCES. Previous marketing framing implied uniform lift across models. Also add the catastrophic-forgetting note to the injection card (replay-via-injection is CLS-grounded and is a safe-to-claim item per the gap-analysis synthesis) and replace prose em dashes with colons/ commas per Oliver voice rules.
… and Min-2022 control The prior +13.2% quality number came from a 10-task single-model run that predated the Min-2022 random-label control. It was ambiguous between content signal and ICL format signal, and it implied general-purpose capability lift rather than preference adherence. Replace with ablation v4: - 4 models x 6 conditions x 16 tasks x 3 iterations, 432 trials - Per-model preference + correctness lift - Min 2022 random-label control included: random-content rules regress 3-10% on 3 of 4 models, so content carries the signal Also tighten the meta-rule paragraph per ablation v3: LLM-synthesized meta-rules add value on smaller models, neutral on larger. Deterministic-template meta-rules regress and are gated out. Prior framing implied uniform cross-domain intelligence lift.
The confidence update in self_improvement.py is a loss-weighted survival score with hand-picked severity bonuses and penalties. It is not a Bayesian posterior. The synthesis in .tmp/gap-analysis/00-SYNTHESIS.md flags 'Bayesian' framing as false advertising until Beta posteriors ship.
|
@coderabbitai review — CR Pro active, please re-verify. |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx (1)
162-174:⚠️ Potential issue | 🟠 MajorAccessible fallback is still not exposed to assistive tech.
Line 167 applies
aria-hidden+hidden, and Lines 169-174 render empty spans, so the declared a11y fallback is non-functional.Proposed fix
- <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> + <div className="sr-only" role="list" aria-label="Graduation markers in selected range"> + {visibleMarkers.map((l) => { + const dateText = l.graduated_at + ? new Date(l.graduated_at).toLocaleDateString('en-US', { + month: 'short', + day: 'numeric', + year: 'numeric', + }) + : 'unknown date' + return ( + <span + key={l.id} + role="listitem" + data-graduation-marker + data-lesson-id={l.id} + data-graduated-at={l.graduated_at ?? ''} + > + {`Lesson ${l.id} graduated on ${dateText}`} + </span> + ) + })} + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx` around lines 162 - 174, The accessible fallback is currently hidden from assistive tech because the wrapper uses aria-hidden and the spans are empty; change the implementation so the fallback remains visually hidden but accessible: remove the aria-hidden attribute and the "hidden" class on the wrapper and instead apply a visually-hidden/sr-only CSS utility; ensure each span rendered from visibleMarkers includes readable content or an aria-label (e.g., include lesson id and graduated_at) while preserving data-graduation-marker and data-lesson-id/data-graduated-at attributes so screen readers can surface the info and tests still find the markers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx`:
- Around line 162-174: The accessible fallback is currently hidden from
assistive tech because the wrapper uses aria-hidden and the spans are empty;
change the implementation so the fallback remains visually hidden but
accessible: remove the aria-hidden attribute and the "hidden" class on the
wrapper and instead apply a visually-hidden/sr-only CSS utility; ensure each
span rendered from visibleMarkers includes readable content or an aria-label
(e.g., include lesson id and graduated_at) while preserving
data-graduation-marker and data-lesson-id/data-graduated-at attributes so screen
readers can surface the info and tests still find the markers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8a6ee21f-5746-43bc-a30f-9c6adab86dd1
📒 Files selected for processing (1)
cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx (3)
3-3: No actionable concern in this import-line change.
73-79: Time-scale axis + tooltip label formatting looks correct.This keeps human-readable date labels while enabling numeric timestamp alignment for overlays.
Also applies to: 100-102
104-120: ReferenceLine marker overlay implementation is solid.Using
x={gMs}on a numeric time X-axis is the right approach for visually aligned graduation markers.
Recharts' Tooltip labelFormatter signature types `label` as ReactNode, so annotating the param as `number` broke `tsc` on Cloudflare Pages (only Next turbopack locally was lenient enough to miss it). Coerce to number at runtime and fall back to String(label) when non-finite. Unblocks the CF Pages build for PR #47. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
CF Pages failure — real build break, now fixedRoot cause: TypeScript error in Next.js (turbopack) dev locally did not surface this, but Cloudflare Pages runs Why not the same pattern as #39: #39 only touched Fix (commit Local verification:
CF Pages should re-run on push; if it still fails with the Supabase env error, that's a CF project config fix, not a blocker for merging this PR on code correctness. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review — re-verify latest pushes |
|
✅ Actions performedFull review triggered. |
…al-markers # Conflicts: # cloud/dashboard/app/(dashboard)/dashboard/page.tsx # cloud/dashboard/src/components/brain/ActiveRulesPanel.tsx # cloud/dashboard/src/components/brain/ActivityFeed.tsx # cloud/dashboard/src/components/brain/CategoriesChart.tsx # cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx # cloud/dashboard/src/components/brain/KpiStrip.tsx # cloud/dashboard/tests/ActiveRulesPanel.test.tsx # cloud/dashboard/tests/ActivityFeed.test.tsx # cloud/dashboard/tests/CategoriesChart.test.tsx # cloud/dashboard/tests/KpiStrip.test.tsx # cloud/dashboard/tests/dashboard-page.test.tsx
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Retargeted to main + rebased after #46 merged |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review — rebased on latest main, all prior fixes applied |
|
✅ Actions performedFull review triggered. |
1 participant
Follow-up to PR #46. Refactors the decay curve chart to render real dashed vertical graduation markers overlaid on the curve, not just a hidden span list.
What changed
XAxisswitched fromdataKey="day"(categorical string) todataKey="ts"withtype="number"+scale="time"+ atickFormatterthat produces the same human label.labelFormatterto the Tooltip so the hover label still reads as a date.<ReferenceLine x={gradMs} stroke="var(--color-accent-blue)" strokeOpacity={0.4} strokeDasharray="4 4" />per visible graduated lesson.buildDecayCurvealready emitted a numerictsfield on each point (alongside the legacydaylabel), so no analytics-client change was needed.[data-graduation-marker]span list preserved as screen-reader fallback + test hook.Test plan
Generated with Gradata