Skip to content

Feat/ens spp accountability#1662

Merged
pikonha merged 23 commits intodevfrom
feat/ens-spp-accountability
Mar 17, 2026
Merged

Feat/ens spp accountability#1662
pikonha merged 23 commits intodevfrom
feat/ens-spp-accountability

Conversation

@edulennert
Copy link
Copy Markdown
Contributor

No description provided.

edulennert and others added 2 commits February 25, 2026 18:24
- Add serviceProviders boolean flag to DaoConfiguration type
- Enable serviceProviders: true in ENS config
- Add serviceProviders entry to PAGES_CONSTANTS
- Add Building2 sidebar nav item in HeaderDAOSidebar (ENS-only)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a new Service Providers page showing quarterly report status for
ENS-funded service providers. Data is static and updated via GitHub PRs.

- types.ts: ReportStatus, QuarterReport, ServiceProvider interfaces + helpers
- data/ens-service-providers.ts: static data for 9 ENS SPPs (2025 & 2026)
- StatusCell: renders Published/Overdue/Due Soon/Upcoming with icons
- ProviderNameCell: name with avatar initials, website + proposal links
- ServiceProvidersTable: full table with current-quarter orange highlight
- CsvExportButton: download table data as CSV
- ServiceProvidersSection: page layout with year tabs, inline alert, update button
- page.tsx: Next.js route guarded by serviceProviders feature flag

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
anticapture Ready Ready Preview, Comment Mar 17, 2026 5:28pm
anticapture-storybook Ready Ready Preview, Comment Mar 17, 2026 5:28pm

Request Review

…anticapture into feat/ens-spp-accountability
@claude
Copy link
Copy Markdown

claude Bot commented Mar 9, 2026

Claude finished @pikonha's task in 2m 3s —— View job


Code Review: ENS Service Providers Accountability

  • Gather context and read changed files
  • Review service providers feature implementation
  • Review shared component changes
  • Review configuration and routing changes
  • Post final review

Overall the feature is well-structured and readable. Below are the issues found, ordered by severity.


🔴 Critical

1. Near-zero cache on file fetches — will flood GitHub's raw CDN

fetchServiceProvidersData.ts:28 uses next: { revalidate: 1 } for fetching individual quarter report files. A one-second revalidation is effectively no caching and will result in a new network request to raw.githubusercontent.com on almost every page view. With 8 providers × 4 quarters × N years this can be 64+ fetches per request. Change this to match the tree fetch (3600) or higher.

Fix this →

2. GitHub API unauthenticated rate limit — 60 req/hour

Neither the tree fetch nor the file fetches pass a Authorization header. GitHub's unauthenticated API rate limit is 60 requests/hour. In production this will be exhausted quickly. At minimum, add a GITHUB_TOKEN env variable and pass it as a Bearer token, or use a server-side proxy.


🟡 Medium

3. Page is ENS-hardcoded regardless of daoId

page.tsx:47 always fetches ENS_SERVICE_PROVIDERS slugs and maps over ENS_SERVICE_PROVIDERS unconditionally. If another DAO config gains serviceProviders: true, it would incorrectly render ENS data. The daoId should control which provider list and fetch function to use, or the mapping should be registered in the DAO config.

// page.tsx line 45-56 — always uses ENS_SERVICE_PROVIDERS regardless of daoId
const serviceProvidersData = await fetchServiceProvidersData(
  ENS_SERVICE_PROVIDERS.map((p) => p.githubSlug),  // ← always ENS
);
const providers = ENS_SERVICE_PROVIDERS.map(...)   // ← always ENS

4. defaultYear can be undefined when API fails

ServiceProvidersSection.tsx:31-33: when availableYears is empty (e.g., the GitHub API call fails and returns {}), availableYears[0] is undefined, and useState(undefined) is passed to <ServiceProvidersTable year={selectedYear} />. This can produce rendering errors downstream. Add a fallback:

const defaultYear = availableYears.includes(currentYear)
  ? currentYear
  : (availableYears[0] ?? currentYear);  // fallback to current year

Fix this →

5. computeQuarterStatus returns misleading "upcoming" for years not in QUARTER_DUE_DATES

computeQuarterStatus.ts:12-13: If the GitHub tree has a year folder that isn't defined in QUARTER_DUE_DATES (e.g., a future 2027 year or a historical 2024), all its unsubmitted quarters silently return "upcoming". Past quarters for unknown years should likely return "overdue".


🔵 Minor / Polish

6. Hardcoded hex colors in ServiceProvidersSection.tsx

Lines 63-64 use raw hex values #3F3F46, #A1A1AA, #27272A for the year-selector buttons instead of design system Tailwind tokens. These won't respond to theme changes and differ from patterns used elsewhere in the codebase.

Fix this →

7. onRowClick={() => {}} creates a confusing UX in ServiceProvidersTable.tsx

Line 152 passes an empty handler to the Table component's onRowClick. If Table applies a pointer cursor or click styles on rows when this prop is present, it creates a misleading interaction. Either remove it or wire it to a meaningful action (e.g., open the report URL).

8. ens-labs.svg added but ENS Labs not in ENS_SERVICE_PROVIDERS

A public/images/ens-labs.svg was committed but no provider entry uses it. If ENS Labs is intentionally excluded from SPP2 (e.g., they are the program manager), a comment explaining this would prevent future confusion.

9. extractUrlFromMarkdown only captures the first link

extractUrlFromMarkdown.ts:2: The regex extracts only the first markdown link in the file. If the convention for .md report files is well-defined (one link per file), this is fine — but worth a comment explaining the expected file format.

10. ProviderRow interface duplicates ServiceProvider fields

ServiceProvidersTable.tsx:21-31: ProviderRow repeats name, iconUrl, websiteUrl, proposalUrl, budget from ServiceProvider. Consider deriving it: type ProviderRow = Omit<ServiceProvider, 'githubSlug' | 'years'> & YearData.


Summary

The feature is functional and well-organized for an initial implementation. The two most important items before going to production are:

  1. Fix the revalidate: 1 cache TTL to avoid hammering GitHub's CDN on every page load.
  2. Add a GitHub API token to avoid hitting the 60 req/hour unauthenticated rate limit in production.

Copy link
Copy Markdown
Collaborator

@isadorable-png isadorable-png left a comment

Choose a reason for hiding this comment

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

Design

  • Remove table row hover (it's not clickable).
  • We don't have a link with the font size of 10px. Please change it to 12px, like our design system and remove the gap between the provider name and the links (on the 2nd screenshot there's an example using Blockful of how it should be). We should also use the component dot divider from the design system, replace the dot for it.
Image Image
  • Change the "Due by XXX XX" font size from 10px to 12px. We don't use 10px on the interface.
  • Also I tested this full orange border on the cell, is it possible to do it? I think it would highlight better. I know we talked about removing the border bottom.
Image

Data

  • I think this repot form ZK email is not from this quarter, but from Q3, and the one that is on 2025/Q3 is of 2025/Q1 and Q2.
Image

Awesome job!

@pikonha pikonha requested a review from brunod-e March 10, 2026 12:57
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

couldn't that be turned into a hook?

@pikonha pikonha merged commit e734874 into dev Mar 17, 2026
5 checks passed
@pikonha pikonha deleted the feat/ens-spp-accountability branch March 17, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants