Feat/risk areas v2

[brunod-e]

No description provided.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
anticapture	Ready	Preview, Comment	Feb 24, 2026 1:17pm
anticapture-storybook	Ready	Preview, Comment	Feb 24, 2026 1:17pm

[edulennert]

@claude review

[claude]

Claude encountered an error —— View job

---

PR Review: Feat/risk areas v2

Tasks

• Gather context and understand PR scope
• Read key changed files
• Analyze code quality, bugs, and patterns
• Post comprehensive review

---

This PR introduces a drawer-based detail view for risk area metrics and a redesigned Resilience Stages section. The previous Claude review had several findings — many were fixed, a few remain, and I found some new issues.

---

Fixed Since Last Review ✅

• scr.ts — $UNI → $SCR token symbol corrected
• gtc.ts — "Optimism DAO" → "Gitcoin DAO" text fixed
• gtc.ts VETO_STRATEGY — riskLevel changed from LOW to MEDIUM (correct, DAO has no veto strategy)
• "Voting Period sarts" typo — fixed across all configs
• Uniswap SPAM_RESISTANCE — correctly LOW now (per-address proposal limit exists)
• titleAbbreviation: "GFR" → "GR" — fixed in risk-areas.ts:87
• Gitcoin SECURITY_COUNCIL, Scroll/Obol/Nouns PROPOSER_BALANCE_CANCEL — correctly absent from those DAO configs, so they don't appear in the UI

---

🔴 Data/Logic Bugs

1. comp.ts — PROPOSER_BALANCE_CANCEL riskLevel contradicts its own description

apps/dashboard/shared/dao-config/comp.ts:299-313

riskLevel: RiskLevel.LOW,
currentSetting: "There is no ability to cancel a proposal if the proposer's balance falls below the Proposal Threshold after submitting it.",
impact: "An attacker can buy tokens to submit a proposal in the DAO, vote with them, and sell during the voting period...",
nextStep: "The parameter is in its lowest-risk condition."

riskLevel: LOW and nextStep: "lowest-risk condition" directly contradict the currentSetting (no cancel ability) and impact (describes an active attack vector). The risk level should be HIGH or MEDIUM and nextStep should describe the fix. This will display incorrect risk data in the drawer. Fix this →

2. createRequirementsFromMetrics.ts:29-35 — type contract violation

return {
  name: metricEnum,
  riskLevel: ...,
  description: field?.description || "",
  // ← currentSetting, impact, recommendedSetting, nextStep are all missing
};

RequirementMetric extends GovernanceImplementationField which declares currentSetting, impact, recommendedSetting, and nextStep — but none of those are populated here. The returned objects silently have undefined for those fields. getSelectedMetricData re-fetches from daoConfig to work around this, but the type is misleading. The return type should either be narrowed (Pick<GovernanceImplementationField, 'riskLevel' | 'description'> & { name: string }) or all fields should be spread. Fix this →

---

🟡 Code Quality Issues (Still Outstanding)

3. Accessibility: empty DrawerPrimitive.Title — Drawer.tsx:66

<DrawerPrimitive.Title></DrawerPrimitive.Title>

Vaul uses this as the aria-labelledby for the drawer. An empty title makes the drawer inaccessible to screen readers. Either pass a title via a prop and render it here (e.g. sr-only), or use VisuallyHidden. Fix this →

4. Misleading color for NONE risk — PendingCriteriaItem.tsx:11

[RiskLevel.NONE]: "text-error",

NONE means data unavailable, not high risk. This maps it to the same red color as HIGH risk. RiskDescription.tsx correctly maps NONE to a HelpCircle secondary icon — PendingCriteriaItem should match that with "text-secondary". Fix this →

5. NEXT_STAGE_CONFIG[Stage.TWO] is copy-paste of Stage.ONE — PendingCriteriaCard.tsx:13

[Stage.ONE]: { label: "Stage 2", colorClass: "text-success" },
[Stage.TWO]: { label: "Stage 2", colorClass: "text-success" },  // ← wrong

Stage 2 is the final stage — there is no next stage. A DAO at Stage 2 should never have pending fields (the calling code passes []), so this never renders in practice, but the config is wrong and could cause confusion if the logic changes. Fix this →

6. Hardcoded external URL — FrameworkOverviewCard.tsx:61

href="https://blockful.gitbook.io/anticapture/anticapture/framework"

Should be extracted to pages-constants.ts or a links.ts constants file for maintainability. Fix this →

7. Timeout cleanup missing — RiskAnalysisSection.tsx:90

The transitionTimeoutRef is cleared when a new click fires (line 104) but not on component unmount. If the component unmounts during the 150ms transition window, the queued state update fires after unmount. Add a cleanup useEffect:

useEffect(() => () => {
  if (transitionTimeoutRef.current) clearTimeout(transitionTimeoutRef.current);
}, []);

Fix this →

8. Bottom drawer conflicting Tailwind classes — Drawer.tsx:59

"data-[vaul-drawer-direction=bottom]:h-full data-[vaul-drawer-direction=bottom]:max-w-[800px]"

h-full makes the bottom drawer full-height (unusual for mobile), and max-w-[800px] constrains width on what is a mobile-only variant. The max-w-[800px] class should be on the right-direction drawer, not bottom. Fix this →

---

🟢 Positive Notes

• useRiskAreaData hook cleanly extracts data concern from display component
• generateRiskDescriptions inside useMemo — correct, prevents unnecessary JSX re-creation
• Ref-preservation pattern in GovernanceImplementationDrawer (lines 59-63) is a clean way to keep content visible during close animation
• RECOMMENDED_SETTINGS constant properly shared across all DAO configs, no per-DAO duplication
• MetricButton non-clickable for NONE risk — correct behavior (unavailable data not interactive)
• getDaoStageFromFields handling of ATTACK_PROFITABILITY — properly excluded when supportsLiquidTreasuryCall is false

---

Note: requirements fields are still populated in many DAO configs, with the comment // Remove this when update Risk Analysis and Stages to not rely on it. Cleanup is still pending from GovernanceImplementationField type.

[edulennert]

@claude review

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[edulennert]

@claude review

[claude]

Claude finished @edulennert's task in 3m 35s —— View job

---

PR Review: Feat/risk areas v2

Tasks

• Gather context and understand PR scope
• Read key changed files and diff since last review
• Analyze outstanding issues from previous reviews
• Check for new issues
• Post comprehensive review

---

This is the third review pass on this PR. The previous review (#2) was approved by @isadorable-png on Feb 20. Several issues flagged in reviews #1 and #2 remain unresolved in the current branch state. One new issue was found.

---

Fixed Since Review #2 ✅

No new fixes were landed since the Feb 20 review — the most recent commits are linter/codegen changes merged from dev. All of the review #2 "fixed" items remain fixed.

---

🔴 Data/Logic Bugs (Still Outstanding)

1. comp.ts:303-318 — PROPOSER_BALANCE_CANCEL riskLevel directly contradicts content

Flagged in both previous reviews, still unresolved:

riskLevel: RiskLevel.LOW,                              // ← says LOW risk
currentSetting: "There is no ability to cancel..."     // ← no protection exists
impact: "An attacker can buy tokens to submit..."      // ← active attack vector
nextStep: "The parameter is in its lowest-risk condition." // ← contradicts everything above

The content is correct (no cancel ability = high risk). The riskLevel should be HIGH or MEDIUM and nextStep needs to describe the required fix, not claim it's already resolved. This bug shows incorrect data in the drawer and stages section. Fix this →

2. comp.ts:87 — INTERFACE_RESILIENCE nextStep says "Nouns" instead of "Compound" (NEW)

nextStep: "Nouns needs to enable DNSSEC and HTTPS on the domains of its governance interfaces..."

This is in the Compound config. The text was copy-pasted from nouns.ts without updating the DAO name. All other DAOs have their correct name in this field (Gitcoin, Optimism, Scroll, Obol). Fix this →

---

🟡 Code Quality Issues (Still Outstanding)

3. Accessibility: empty DrawerPrimitive.Title — Drawer.tsx:67

<DrawerPrimitive.Title></DrawerPrimitive.Title>

Vaul uses this as aria-labelledby for the drawer. Leaving it empty makes the drawer inaccessible to screen readers. Add a visually hidden title or accept a title prop. Fix this →

4. Misleading color for RiskLevel.NONE — PendingCriteriaItem.tsx:12

[RiskLevel.NONE]: "text-error",   // ← red, same as HIGH risk

NONE means "data unavailable", not high risk. RiskDescription.tsx correctly maps NONE to a secondary HelpCircle icon. PendingCriteriaItem should match with "text-secondary". Fix this →

5. NEXT_STAGE_CONFIG[Stage.TWO] is copy-paste of Stage.ONE — PendingCriteriaCard.tsx:11

[Stage.ONE]: { label: "Stage 2", colorClass: "text-success" },
[Stage.TWO]: { label: "Stage 2", colorClass: "text-success" },  // ← wrong, Stage 2 is final

Stage 2 is the final stage — it has no "next stage". The pendingFields for Stage 2 DAOs is always [] so this never renders in practice, but the config is misleading and would silently display wrong text if the logic changes. Fix this →

6. Hardcoded external URL — FrameworkOverviewCard.tsx:62

href="https://blockful.gitbook.io/anticapture/anticapture/framework"

Should be extracted to a named constant in pages-constants.ts or a links.ts file for maintainability. Fix this →

7. Timeout cleanup missing on unmount — RiskAnalysisSection.tsx

transitionTimeoutRef is cleared when a new click fires (line 105) but not on unmount. If the component unmounts during the 150ms transition window, the queued setState fires after unmount:

// Add this alongside the other useEffects:
useEffect(() => () => {
  if (transitionTimeoutRef.current) clearTimeout(transitionTimeoutRef.current);
}, []);

Fix this →

8. Bottom drawer conflicting Tailwind classes — Drawer.tsx:60

"data-[vaul-drawer-direction=bottom]:h-full data-[vaul-drawer-direction=bottom]:max-w-[800px]"

max-w-[800px] has no effect on a full-screen bottom drawer and likely belongs on the right-direction drawer instead. Fix this →

9. createRequirementsFromMetrics.ts type contract violation — still present

The function returns objects typed as RequirementMetric (which extends GovernanceImplementationField) but only populates name, riskLevel, and description. The other fields (currentSetting, impact, recommendedSetting, nextStep) are undefined. getSelectedMetricData works around this by re-fetching from daoConfig, but the type is misleading. The return type should be narrowed to only what's actually set.

---

🟢 Positive Notes

• Ref-preservation pattern in GovernanceImplementationDrawer (lines 59-64) correctly keeps content visible during close animation.
• MetricButton non-clickable for NONE risk — correct behavior.
• generateRiskDescriptions inside useMemo — prevents unnecessary JSX re-creation.
• RECOMMENDED_SETTINGS shared constant properly eliminates per-DAO duplication.
• getDaoStageFromFields correctly excludes ATTACK_PROFITABILITY when supportsLiquidTreasuryCall is false.
• ResilienceStagesSection correctly filters out ATTACK_PROFITABILITY from visible fields before passing to drawer (so the NONE-riskLevel edge case from createRequirementsFromMetrics doesn't affect the stages section).

---

Summary of outstanding items: 2 RED bugs (PROPOSER_BALANCE_CANCEL data contradiction in comp.ts, wrong DAO name "Nouns" in Compound's INTERFACE_RESILIENCE nextStep), 7 YELLOW code quality items. The most impactful to fix before merging are issues #1 and #2 which produce incorrect data visible to users.