Introducing admin actions to handle disqualifications and warnings#1943
Introducing admin actions to handle disqualifications and warnings#1943
Conversation
🦋 Changeset detectedLatest commit: 5bae4b2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
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 2 minutes and 1 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 (3)
📝 WalkthroughWalkthroughRefactors rev-share-cap to replace Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the rev-share-cap referral rules/data model to support both disqualifications and warnings as “admin actions”, aligning the backend with the refined referral rules model described in #1906.
Changes:
- Introduces
AdminActionas a discriminated union (Disqualification | Warning) and renames rules fielddisqualifications→adminActions. - Updates metrics to carry
adminActioninstead ofadminDisqualificationReason, while keepingisAdminDisqualifiedderived from the action type. - Updates API zod schemas + serialization and extends leaderboard tests to cover warnings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/award-models/rev-share-cap/rules.ts | Adds admin action types/union, renames disqualifications → adminActions, updates validation + qualification logic. |
| packages/ens-referrals/src/award-models/rev-share-cap/metrics.ts | Propagates adminAction into ranked/unranked metrics and validates consistency with rules. |
| packages/ens-referrals/src/award-models/rev-share-cap/leaderboard.test.ts | Updates fixtures/assertions for adminActions and adds warning-specific test coverage. |
| packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts | Adds admin action schemas and updates rev-share-cap rules/metrics schemas accordingly. |
| packages/ens-referrals/src/award-models/rev-share-cap/api/serialize.ts | Serializes adminActions and adminAction fields in API payloads. |
| packages/ens-referrals/src/api/zod-schemas.test.ts | Updates rev-share-cap metrics parsing fixture to use adminAction. |
| .changeset/sharp-wolves-march.md | Declares a minor release for the new admin actions model and renamed fields. |
Comments suppressed due to low confidence (1)
packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts:83
makeReferralProgramRulesRevShareCapSchemadefaultsadminActionsto[]. Since Zod objects strip unknown keys by default, an older payload that still sendsdisqualificationswill parse successfully but silently drop the disqualifications (treating them as no admin actions). Consider either (a) adding backward-compatible parsing for a legacydisqualificationsfield (mapping it toadminActions), or (b) makingadminActionsrequired (remove.default([])) so outdated payloads fail loudly instead of being misinterpreted.
adminActions: z
.array(makeAdminActionSchema(`${valueLabel}.adminActions[item]`))
// NOTE: addresses are already normalized, so string equivalence here is accurate
.refine(
(items) => {
const referrers = items.map((a) => a.referrer);
return new Set(referrers).size === referrers.length;
},
{
message: `${valueLabel}.adminActions must not contain duplicate referrer addresses`,
},
)
.default([]),
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR introduces Confidence Score: 5/5Safe to merge — all findings are non-blocking P2 suggestions with no logic or data-integrity defects introduced. The implementation is well-structured with a clean discriminated union, thorough runtime validators, documented invariants, and comprehensive leaderboard tests for disqualification/warning scenarios. All three findings are P2: a missing cross-field Zod refine, missing non-null adminAction test cases in the API schema test file, and a minor whitespace normalization inconsistency between the build and Zod parse paths. None of these affects correctness of data produced by the server. packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts (missing referrer cross-validation) and packages/ens-referrals/src/api/zod-schemas.test.ts (missing non-null adminAction test cases) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ReferralEvent processed] --> B{Is referrer in adminActions?}
B -- Yes, Disqualification --> C[isAdminDisqualified = true\nadminAction = DisqualificationAction]
B -- Yes, Warning --> D[isAdminDisqualified = false\nadminAction = WarningAction]
B -- No --> E[isAdminDisqualified = false\nadminAction = null]
C --> F{Met revenue threshold?}
D --> F
E --> F
F -- Yes AND not disqualified --> G[isQualified = true\nclaim from awardPool]
F -- Yes BUT disqualified --> H[isQualified = false\ncappedAward = 0\npool preserved]
F -- No --> I[isQualified = false\ncappedAward = 0]
G --> J[buildAwardedReferrerMetricsRevShareCap]
H --> J
I --> J
J --> K[validateAwardedReferrerMetricsRevShareCap\nEnforces: isAdminDisqualified ↔ adminAction.actionType\nEnforces: cappedAward = 0 when disqualified]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @invariant When true, {@link isQualified} is false. | ||
| * @invariant true if and only if {@link adminAction} has `actionType === "Disqualification"`. | ||
| */ | ||
| isAdminDisqualified: boolean; |
There was a problem hiding this comment.
isAdminDisqualified is fully derivable from adminAction?.actionType === Disqualification, so keeping both fields introduces duplicated state that has to be kept consistent via extra validation/refinements (and creates another surface for API consumers to get wrong). Consider removing isAdminDisqualified from the metrics model/API and deriving it from adminAction where needed (or only keeping it if you need it for backwards compatibility).
There was a problem hiding this comment.
@Goader This seems like a good feedback from Copilot. I believe we can remove the isAdminDisqualified field and instead derive it from adminAction.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Looks good! Shared a few suggestions. Please take the lead to merge when ready 👍
|
|
||
| /** | ||
| * An admin-imposed disqualification entry of a specific referrer in an edition. | ||
| * The types of admin actions that can be imposed on a referrer in a rev-share-cap edition. |
There was a problem hiding this comment.
| * The types of admin actions that can be imposed on a referrer in a rev-share-cap edition. | |
| * The types of admin actions that can be taken upon a referrer in a rev-share-cap edition. |
| Disqualification: "Disqualification", | ||
|
|
||
| /** | ||
| * The referrer is flagged but still eligible for awards. |
There was a problem hiding this comment.
| * The referrer is flagged but still eligible for awards. | |
| * The referrer is warned about a potential disqualification but may still be qualified for awards. |
| export const AdminActionTypes = { | ||
| /** | ||
| * The Ethereum address of the disqualified referrer, as a {@link NormalizedAddress}. | ||
| * The referrer is ineligible for awards. |
There was a problem hiding this comment.
| * The referrer is ineligible for awards. | |
| * The referrer is disqualified for awards. |
| * An admin-imposed disqualification of a specific referrer in an edition. | ||
| * Disqualified referrers receive no awards. |
There was a problem hiding this comment.
| * An admin-imposed disqualification of a specific referrer in an edition. | |
| * Disqualified referrers receive no awards. | |
| * An admin action to disqualify a referrer from receiving awards for an edition. |
| * An admin-imposed warning for a specific referrer in an edition. | ||
| * Warned referrers are still eligible for awards. |
There was a problem hiding this comment.
| * An admin-imposed warning for a specific referrer in an edition. | |
| * Warned referrers are still eligible for awards. | |
| * An admin action to warn a referrer that their eligibility for receiving awards for an edition is at risk unless the referrer takes corrective actions. |
| /** | ||
| * Admin-imposed disqualifications for this edition. | ||
| * Disqualified referrers receive no awards. | ||
| * Admin-imposed actions for this edition. |
There was a problem hiding this comment.
| * Admin-imposed actions for this edition. | |
| * Admin actions for this edition. |
| * Admin-imposed disqualifications for this edition. | ||
| * Disqualified referrers receive no awards. | ||
| * Admin-imposed actions for this edition. | ||
| * Disqualified referrers receive no awards. Warned referrers are still eligible. |
There was a problem hiding this comment.
| * Disqualified referrers receive no awards. Warned referrers are still eligible. |
We can remove this line completely. Each action is documented in more detail elsewhere 👍
| * @invariant When true, {@link isQualified} is false. | ||
| * @invariant true if and only if {@link adminAction} has `actionType === "Disqualification"`. | ||
| */ | ||
| isAdminDisqualified: boolean; |
There was a problem hiding this comment.
@Goader This seems like a good feedback from Copilot. I believe we can remove the isAdminDisqualified field and instead derive it from adminAction.
|
|
||
| /** | ||
| * The reason for admin disqualification, or null if not disqualified. | ||
| * The admin action imposed on this referrer, or null if no action exists. |
There was a problem hiding this comment.
| * The admin action imposed on this referrer, or null if no action exists. | |
| * The admin action taken on this referrer, or null if no admin action has been taken. |
| * | ||
| * @invariant null when {@link isAdminDisqualified} is false. | ||
| * @invariant Non-empty string when {@link isAdminDisqualified} is true. | ||
| * @invariant null when no admin action exists for this referrer. |
There was a problem hiding this comment.
| * @invariant null when no admin action exists for this referrer. | |
| * @invariant null when no admin action has been taken on this referrer. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts (1)
198-220:⚠️ Potential issue | 🟡 MinorZod schemas don't enforce
metrics.adminAction⇔rules.adminActionsconsistency.
validateRankedReferrerMetricsRevShareCap/validateUnrankedReferrerMetricsRevShareCaprequire thatmetrics.adminActionmatches the corresponding entry inrules.adminActions(byactionType,referrer, andreason), but the zod schemas forReferrerEditionMetricsRankedRevShareCap,ReferrerEditionMetricsUnrankedRevShareCap, andReferrerLeaderboardPageRevShareCapdo not replicate that check. Payloads in whichreferrer.adminActionis aDisqualificationwith a reason that differs from (or is missing altogether in)rules.adminActionswill pass schema validation at the API boundary.Consider adding a
.superRefineon each of these edition-level schemas that mirrors the runtime validator, so clients deserializing viaparseget the same guarantees that in-process constructors do.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts` around lines 198 - 220, Add a .superRefine to each edition-level schema (makeReferrerEditionMetricsRankedRevShareCapSchema, makeReferrerEditionMetricsUnrankedRevShareCapSchema, makeReferrerLeaderboardPageRevShareCapSchema) that enforces the same adminAction ↔ rules.adminActions consistency as the runtime validators (validateRankedReferrerMetricsRevShareCap / validateUnrankedReferrerMetricsRevShareCap): for each payload, if metrics.referrer.adminAction is present find the corresponding entry in rules.adminActions by matching actionType and referrer identifier and ensure the adminAction.reason (and presence) matches the rules entry; if no matching rule or the reason differs, call ctx.addIssue with a clear message and path like ["referrer","adminAction"] so zod.parse will reject inconsistent payloads.
♻️ Duplicate comments (1)
packages/ens-referrals/src/award-models/rev-share-cap/metrics.ts (1)
93-99: 🧹 Nitpick | 🔵 TrivialRedundant
@invariantrestating the field description.The first
@invariant(line 96) just restates the summary on line 94 ("null when no admin action..."). Per the coding guideline "Do not add JSDoc@returnstags that merely restate the method summary; remove such redundancy during PR review", drop the redundant invariant and keep only the second one documenting the non-trivial constraint./** * The admin action taken on this referrer, or null if no admin action has been taken. * - * `@invariant` null when no admin action has been taken on this referrer. * `@invariant` Must match the corresponding entry in {`@link` ReferralProgramRulesRevShareCap.adminActions}. */ adminAction: AdminAction | null;As per coding guidelines: "Do not add JSDoc
@returnstags that merely restate the method summary; remove such redundancy during PR review".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ens-referrals/src/award-models/rev-share-cap/metrics.ts` around lines 93 - 99, Remove the redundant JSDoc invariant that repeats the field summary for adminAction; keep the non-trivial invariant that it must match ReferralProgramRulesRevShareCap.adminActions. Specifically, update the JSDoc for the adminAction field (type AdminAction | null) by deleting the invariant "null when no admin action has been taken on this referrer" and retaining the invariant describing the correspondence with ReferralProgramRulesRevShareCap.adminActions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ens-referrals/src/award-models/rev-share-cap/metrics.ts`:
- Around line 120-140: Extract the duplicated adminAction validation into a
shared helper (e.g., validateAdminActionConsistency) that takes
(metricsAdminAction: AdminAction | null, referrer: NormalizedAddress, rules:
ReferralProgramRulesRevShareCap, context: string) and implements the current
checks: find expected = rules.adminActions.find(a => a.referrer === referrer) ??
null, throw the same context-prefixed errors when expected is null but
metricsAdminAction is not, or when expected is non-null but metricsAdminAction
is null or any of actionType/referrer/reason mismatch; then replace the
duplicated blocks in RankedReferrerMetricsRevShareCap validation and
validateUnrankedReferrerMetricsRevShareCap to call this helper with the
appropriate context string to preserve error prefixes.
---
Outside diff comments:
In `@packages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.ts`:
- Around line 198-220: Add a .superRefine to each edition-level schema
(makeReferrerEditionMetricsRankedRevShareCapSchema,
makeReferrerEditionMetricsUnrankedRevShareCapSchema,
makeReferrerLeaderboardPageRevShareCapSchema) that enforces the same adminAction
↔ rules.adminActions consistency as the runtime validators
(validateRankedReferrerMetricsRevShareCap /
validateUnrankedReferrerMetricsRevShareCap): for each payload, if
metrics.referrer.adminAction is present find the corresponding entry in
rules.adminActions by matching actionType and referrer identifier and ensure the
adminAction.reason (and presence) matches the rules entry; if no matching rule
or the reason differs, call ctx.addIssue with a clear message and path like
["referrer","adminAction"] so zod.parse will reject inconsistent payloads.
---
Duplicate comments:
In `@packages/ens-referrals/src/award-models/rev-share-cap/metrics.ts`:
- Around line 93-99: Remove the redundant JSDoc invariant that repeats the field
summary for adminAction; keep the non-trivial invariant that it must match
ReferralProgramRulesRevShareCap.adminActions. Specifically, update the JSDoc for
the adminAction field (type AdminAction | null) by deleting the invariant "null
when no admin action has been taken on this referrer" and retaining the
invariant describing the correspondence with
ReferralProgramRulesRevShareCap.adminActions.
🪄 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: 2a0b1a5d-5a8c-4049-b243-35923ac9a5b0
📒 Files selected for processing (6)
packages/ens-referrals/src/api/zod-schemas.test.tspackages/ens-referrals/src/award-models/rev-share-cap/api/serialize.tspackages/ens-referrals/src/award-models/rev-share-cap/api/zod-schemas.tspackages/ens-referrals/src/award-models/rev-share-cap/leaderboard.test.tspackages/ens-referrals/src/award-models/rev-share-cap/metrics.tspackages/ens-referrals/src/award-models/rev-share-cap/rules.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introducing admin actions to handle disqualifications and warnings
closes: #1906
Summary
AdminActionTypes,AdminActionDisqualification,AdminActionWarning, andAdminActionrev-share-caprules now have an array ofAdminActionAdminActionas part of its modelWhy
Testing
Notes for Reviewer
isAdminDisqualifiedandadminActionin theRankedReferrerMetricsRevShareCap. This duplicates the information about disqualification, but to get the information whether a referrer is disqualified easier. I'd appreciate your advice on whether we should removeisAdminDisqualified.Pre-Review Checklist (Blocking)