Skip to content

3-5 New Charts - #4

Closed
ekoslow1-creator wants to merge 54 commits into
mainfrom
feature/sc-46083/create-3-5-charts
Closed

3-5 New Charts#4
ekoslow1-creator wants to merge 54 commits into
mainfrom
feature/sc-46083/create-3-5-charts

Conversation

@ekoslow1-creator

Copy link
Copy Markdown
Collaborator

New charts for the dashboard

saengel and others added 30 commits June 30, 2026 09:01
Documents the component architecture, live API integration
(sefaria.org/api/powered-by), and the project_category data-quality
handling agreed on during brainstorming.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The API's project_category field decomposes against 5 real form
labels (one legacy naming variant) plus freeform "Other:" text -
not the 6 categories shown on developers.sefaria.org as originally
assumed. Verified by decomposing all 36 live entries with zero
unmatched leftovers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sefaria Intern and others added 22 commits July 28, 2026 10:17
Pre-existing uncommitted work from the primary checkout (tabbed sidebar
dashboard, initial Charts and Analytics bar charts, keyword/submissions-trend
utils, recharts dependency), captured as a single baseline commit so the
experience-level trend chart plan's tasks diff cleanly on top of it. Not part
of this plan's scope.
…ience-level labels render and aren't clipped
…r bucketing

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…pty experience trend

- getToolUsageCounts now filters sefaria_tools_used to string entries before
  normalizing, so a non-string entry (null, number, etc.) in this free-text
  user-submitted field is skipped instead of throwing and blanking out all
  four Charts & Analytics charts via the outer catch handler.
- Move the experienceTrend.length === 0 empty-state message inline (matching
  the pie chart's own conditional-render guard) instead of an early return,
  so the two bar charts and the pie chart still render when there's no
  experience-level data.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Shares VIBE_CODED_SERIES between submissionsTrend.js and
ChartsAndAnalytics.jsx (mirroring the EXPERIENCE_LEVELS precedent)
instead of hardcoding the strings twice, and adds a one-sentence
caption noting vibe_coded is a newly-tracked field so the chart isn't
misread as a real overnight trend reversal.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extracts known technology names from the free-text tech_used_raw field
via case-insensitive substring matching, with "Claude Code" checked and
excluded first so it doesn't double-count toward the generic
Claude/Anthropic API bucket. Shows the top 8 by project count as a
horizontal bar chart, matching the existing keyword-frequency chart's style.
Reuses the fixed-order categorical hue palette already used in the
charts (blue/orange/aqua/violet/magenta, gray for Uncategorized).
Background and border are computed via color-mix() at low strength
so each pill stays pale; text is mixed toward --text-h for a muted
look instead of a fully saturated hue.
…chart

Recharts' category axis auto-hides tick labels it calculates would
overlap; interval={0} disables that, and the wider axis column gives
longer labels like "Claude/Anthropic API" room to fit.
@ekoslow1-creator
ekoslow1-creator requested a review from saengel July 29, 2026 06:29
@gitvelocity-reviewer

Copy link
Copy Markdown

📊 Code Quality Score: 42/100

52 × 0.8 = 41.6 ≈ 42

Category Score Factors
🔭 Scope 13/20 12 files across components, CSS, and 4 utility modules; new user-facing charts; cross-cutting data→component→style layers
🏗️ Architecture 8/20 New utility modules follow established patterns; no new external dependencies; clean data/presentation separation; no new service boundaries
⚙️ Implementation 11/20 normalizeEndpoint longest-match-first algorithm; getTechCounts special-case stripping before pattern matching; CSS color-mix cascade; recharts animation workaround; endpoint deduplication per project
⚠️ Risk 5/20 Additive changes only; no data mutations, auth, or migrations; CSS margin-left hardcoded calculation is fragile but low blast radius
✅ Quality 13/15 Comprehensive tests >90% coverage on all new utility modules; edge cases covered (empty, non-string, boundary at 6, deduplication); good inline comments explaining non-obvious decisions
🔒 Perf / Security 2/5 Other bucket prevents unbounded pie chart rendering; no security concerns; no benchmarks or explicit perf work needed at this scale

Was this score accurate? 👍 Yes · 👎 No

Scored by GitVelocity · How are scores calculated?

@saengel saengel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall, fantastic work! Some comments and learning opportunities - happy to review together when we meet next.

Comment thread src/components/ChartsAndAnalytics.jsx Outdated

// Fixed-order categorical hues; gray is reserved for the "Other" bucket and
// is never one of the 6 identity colors.
const TOOL_SLICE_COLORS = ['#2a78d6', '#eb6834', '#1baf7a', '#eda100', '#e87ba4', '#008300']

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do these match the Sefaria-specific color palette? Worth a quick check (for all of the colors used)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@ekoslow1-creator was this addressed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I took a look at the code, the colors it uses are the Sefaria colors. I wonder if it doesn't look quite right because the category labels (on each ProjectCard) are different colors for uniqueness. Should I play around with those labels and try to do blues only that are similar to the Sefaria website?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah ok, to clarify - let's have all of the colors on the site be pulled from the Sefaria color palette, does that make sense?

Comment thread src/components/ChartsAndAnalytics.jsx Outdated
</ResponsiveContainer>

<h2>Submissions by experience level</h2>
{experienceTrend.length === 0 ? (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Each chart should be scoped into its own wrapper React component and JSX file, and then rendered here to make the code more readable, organized and reusable.

(Learn more about this practice here)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed this, will submit in new pull request

Comment thread src/components/ChartsAndAnalytics.jsx Outdated
label={(props) =>
props.index === experienceTrend.length - 1 ? (
<text
x={props.x + 6}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's going on with the calculations here? Why is it necessary to add 6?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The +6 just offsets the last label 6 pixels to the right of the dot so that it's not overlapping with the dot

Comment thread src/components/ChartsAndAnalytics.jsx Outdated

<h2>Most-used Sefaria API endpoints</h2>
{toolUsage.length === 0 ? (
<p>No endpoint data available yet.</p>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd change this text (everywhere on the page) to Data unavailable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment thread src/components/ChartsAndAnalytics.jsx Outdated

<h2>Vibe-coded vs. not, past 12 months</h2>
<p>
"Vibe-coded" is a newly-tracked field, so earlier months may be undercounted or unreported rather than confirmed non-vibe-coded.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a helpful caveat

Comment thread src/components/ChartsAndAnalytics.jsx Outdated
</ResponsiveContainer>
)}

<h2>Vibe-coded vs. not, past 12 months</h2>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

More accurate to say tracked since July 2026 since we unfortunately don't have it explicitly from much earlier

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@@ -1,3 +1,5 @@
import { getCategoryColor } from '../utils/categories.js'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is a great example of a small, composable component

@@ -0,0 +1,21 @@
import { test } from 'node:test'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wonderful that you added tests - always important. Let's reassess the best testing framework etc soon, we use Playwright on Sefaria-Project, might be worth aligning.

Comment thread src/index.css
@@ -1,2 +1,2 @@
:root {
/* Sefaria brand palette */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would double check, not sure Claude got it right. Let's also make this the one source of truth for the colors if possible and import from here as much as possible.

Comment thread src/utils/techUsed.js
// get counted toward the generic Claude/Anthropic API bucket.
const CLAUDE_CODE_PATTERN = 'claude code'

export const KNOWN_TECHNOLOGIES = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This probably needs to be more robust to be useful in the future, might be a bit too fitted to the current data - but we can leave for now and remove in the future/adapt as needed.

Extracts the six recharts blocks out of ChartsAndAnalytics.jsx into
dedicated presentational components (one file per chart), each owning
its chart-specific colors/constants. ChartsAndAnalytics.jsx now only
handles data fetching/derivation and renders each chart with props.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@saengel saengel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Still needs a few more changes - one in specific is very important, will follow up in a message.

Comment thread data/powered-by-projects-complete.csv Outdated
@@ -0,0 +1,295 @@
id,submission_date,created_at,updated_at,submission_source,sefaria_tools_used,tech_used_raw,technical_experience,vibe_coded,project_why,project_name,project_link,project_source_code,project_reach,project_desc,project_category,image_url,has_pbs_logo,tags,status,is_buggy,last_checked,consent_to_display,is_published,featured

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We shouldn't be committing this file for two reasons:

  1. It contains information we aren't publicly displaying (as per the form) and we need to be careful it's not available to the public, even via GitHub/Git history
  2. It will ultimately live in the database which will be the source of truth - and we don't want this to get confusing.

Please remove this file from the PR, and there's a way to push (not remembering exactly off the top of my head) that wipes the history for the PR so this file is not longer publicly available.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These broken down charts are a step in the right direction, I'm wondering if we can generalize them even more, have 1 bar chart component, 1 line chart etc, and then pass in the data/params as needed to configure for each specific data series we're trying to display.

@@ -1,32 +1,27 @@
import { useEffect, useState } from 'react'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Much better!

@ekoslow1-creator
ekoslow1-creator force-pushed the feature/sc-46083/create-3-5-charts branch from 04d04ce to 165797e Compare August 4, 2026 11:44
@ekoslow1-creator ekoslow1-creator mentioned this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants