feat(contributors): add pagination, sorting, filtering, credits system#869
feat(contributors): add pagination, sorting, filtering, credits system#869bturcotte520 wants to merge 1 commit intomainfrom
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| }, | ||
| user: { | ||
| login: 'brianturcotte', | ||
| id: 999002, |
There was a problem hiding this comment.
[CRITICAL]: brianturcotte is missing from src/data/contributor-champion-kilo-team.json
This test expects brianturcotte to be filtered out as a team member (fetchedMergedPullRequests asserted as 1, not 2). But brianturcotte is not in the team JSON file, so TEAM_LOGIN_SET won't contain it. This means:
- Both PRs (7101 and 7102) will pass the team-login filter in
listMergedPullRequestsSince getPullRequestAuthorEmailwill be called for PR 7102, but the mock doesn't handle/pulls/7102/commits- The mock throws
Error('Unexpected GitHub request in test: ...'), crashing the test
Either add brianturcotte to the team JSON file, or update the mock to handle the 7102 commits endpoint.
| export async function enrollContributorChampion(input: { | ||
| contributorId: string; | ||
| tier: ContributorTier | null; | ||
| adminUser?: User; |
There was a problem hiding this comment.
[WARNING]: adminUser parameter is accepted but never used
The adminUser field is passed from the router (ctx.user) but is never referenced in the function body. The credit grant uses the linked user (the credit recipient), not the admin. Per the coding style guide: "Every function argument has a small cost; add them only where they meaningfully simplify the caller somehow." Remove this unused parameter.
src/app/admin/contributors/page.tsx
Outdated
| onValueChange={value => | ||
| setEnrolledFilters(prev => ({ | ||
| ...prev, | ||
| hasKiloAccount: value as KiloAccountFilter, |
There was a problem hiding this comment.
[WARNING]: value as KiloAccountFilter uses TypeScript's as operator
Per coding style: avoid as casts because they cannot be statically checked. This same pattern also appears on line 819. Consider a runtime validation helper instead:
function toKiloAccountFilter(value: string): KiloAccountFilter {
if (value === 'all' || value === 'yes' || value === 'no') return value;
return 'all';
}Then use hasKiloAccount: toKiloAccountFilter(value) in both places.
src/app/admin/contributors/page.tsx
Outdated
|
|
||
| setHasAutoSynced(true); | ||
| void syncMutation.mutateAsync(); | ||
| }, [hasAutoSynced, syncMutation]); |
There was a problem hiding this comment.
[WARNING]: syncMutation in the useEffect dependency array is an unstable reference
useMutation returns a new object on every render. While the hasAutoSynced guard prevents an infinite loop, syncMutation as a dependency causes this effect to re-evaluate on every render. This is unnecessarily wasteful. Consider using syncMutation.mutateAsync directly (stable function reference) or extracting the mutate function via a ref.
| expect(afterEnrollment?.tier).toBe('ambassador'); | ||
| expect(afterEnrollment?.enrolledAt).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[WARNING]: No test coverage for refreshContributorChampionCredits or processAutoTierUpgrades
These are two critical service functions (credit granting and tier auto-upgrades) that are called by the cron job but have zero test coverage. Both have non-trivial logic (30-day rolling checks, tier threshold comparisons, DB updates) that should be tested. At minimum:
- Test that
refreshContributorChampionCreditsonly grants to eligible memberships (30+ days elapsed, linked user exists) - Test that
processAutoTierUpgradescorrectly upgrades tiers at the 5-PR and 15-PR thresholds - Test edge cases: already at highest tier, no linked user, credit grant failure
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE "contributor_champion_contributors" ADD COLUMN "manual_email" text;--> statement-breakpoint | |||
There was a problem hiding this comment.
[WARNING]: Schema drift — manual_email column added by this migration but missing from the Drizzle schema definition
Migration 0046 adds manual_email text and an index on contributor_champion_contributors, but the Drizzle schema in packages/db/src/schema.ts (lines 1998-2023) does not include this column. This means the column exists in the DB but is invisible to the ORM — it can't be queried, inserted, or updated through Drizzle. If this column is intended for future use, add a comment in the migration noting that. If it should be usable now, add it to the schema definition.
Code Review SummaryStatus: 33 Issues Found | Recommendation: Address critical issues before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)CRITICAL
WARNING (highlights)
SUGGESTION
Other Observations (not in diff)N/A — all issues are on new code within the diff. Files Reviewed (20 files)
|
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
558d058 to
0c96df0
Compare
| SELECT m.contributor_id FROM contributor_champion_memberships m | ||
| WHERE m.linked_kilo_user_id = ${userId} | ||
| )` | ||
| ); |
There was a problem hiding this comment.
[CRITICAL]: GDPR soft-delete only clears emails for events linked through contributor_champion_memberships.linked_kilo_user_id, but misses events where github_author_email matches the user's email without a membership link.
If a contributor has events with the user's email but was never enrolled (no membership row), or the membership doesn't have linked_kilo_user_id set, those email addresses survive deletion. For GDPR compliance, you should also clear emails matched directly:
// Also clear events matched by email directly (covers un-enrolled contributors)
await tx
.update(contributor_champion_events)
.set({ github_author_email: null })
.where(
sql`lower(${contributor_champion_events.github_author_email}) = lower(
(SELECT u.google_user_email FROM kilocode_users u WHERE u.id = ${userId})
)`
);| const currentIndex = nextIndex; | ||
| nextIndex += 1; | ||
| const value = values[currentIndex]; | ||
| if (!value) return; |
There was a problem hiding this comment.
[WARNING]: !value uses a falsy check for worker termination. This works for arrays of objects but would silently skip items if any element is falsy (0, '', null). Since the function is generic (<T, TResult>), a caller passing e.g. [0, 1, 2] would only process [1, 2].
Use an index boundary check instead:
| if (!value) return; | |
| if (currentIndex >= values.length) return; |
| contributorId: string; | ||
| tier: ContributorTier | null; | ||
| }): Promise<EnrollResult> { | ||
| const leaderboard = await getContributorChampionLeaderboard(); |
There was a problem hiding this comment.
[WARNING]: enrollContributorChampion fetches the entire leaderboard (with multiple CTEs and JOINs across all contributors) just to .find() a single row. Same pattern in getContributorChampionReviewQueue() (line 675) and getEnrolledContributorChampions() (line 766) which also fetch the full leaderboard and filter in-memory.
This will become a performance bottleneck as the contributor count grows. Consider adding a targeted query that looks up a single contributor by ID for enrollment, or at minimum adding pagination/LIMIT to the leaderboard query for filtered use cases.
|
|
||
| function parseContributorTier(value: string | null): ContributorTier | null { | ||
| if (value === null) return null; | ||
| return CONTRIBUTOR_TIER_SCHEMA.parse(value); |
There was a problem hiding this comment.
[WARNING]: CONTRIBUTOR_TIER_SCHEMA.parse() throws a Zod ZodError on invalid input. This function is called in processAutoTierUpgrades (line 875) inside a loop — if any DB row has corrupted tier data, the entire auto-upgrade cron step crashes.
The function's return type (ContributorTier | null) and name suggest it should handle invalid input gracefully. Use safeParse instead:
| return CONTRIBUTOR_TIER_SCHEMA.parse(value); | |
| const result = CONTRIBUTOR_TIER_SCHEMA.safeParse(value); | |
| return result.success ? result.data : null; |
bturcotte520
force-pushed
the
brian/addContributorDash
branch
3 times, most recently
from
b9f52dd to
639c923
Compare
| const kiloUser = kiloUserRows.rows[0]; | ||
| if (kiloUser) { | ||
| creditGranted = await grantContributorChampionCredits( | ||
| { id: kiloUser.id } as User, |
There was a problem hiding this comment.
[CRITICAL]: { id: kiloUser.id } as User passes an incomplete User object to grantCreditForCategory
grantCreditForCategoryConfig (promotionalCredits.ts:171-178) reads user.microdollars_used to populate expiration_baseline_microdollars_used and original_baseline_microdollars_used on the credit transaction row. With only { id } provided, both values will be undefined, breaking the credit expiration system — processLocalExpirations() relies on these baselines to determine when credits are consumed.
It also reads user.google_user_email (line 155) for logging, which will also be undefined.
Compare with enrollContributorChampion (line 721-725) which correctly does db.select().from(kilocode_users) to fetch the full user record. Use the same pattern here:
| { id: kiloUser.id } as User, | |
| creditGranted = await grantContributorChampionCredits( | |
| (await db.select().from(kilocode_users).where(eq(kilocode_users.id, kiloUser.id)).limit(1))[0]!, | |
| creditAmountUsd | |
| ); |
Or better yet, reuse the already-fetched kiloUser row by selecting all columns in the query at line 1043.
| enrolled_at: now, | ||
| credit_amount_microdollars: creditAmountMicrodollars, | ||
| linked_kilo_user_id: input.kiloUserId, | ||
| credits_last_granted_at: creditAmountUsd > 0 && input.kiloUserId ? now : null, |
There was a problem hiding this comment.
[WARNING]: credits_last_granted_at is persisted before credits are actually granted
The membership row is inserted here with credits_last_granted_at: now, but the actual credit grant doesn't happen until lines 1042-1051. If grantContributorChampionCredits fails or returns false, the DB incorrectly records that credits were granted.
Additionally, the onConflictDoUpdate clause (lines 1030-1037) omits credits_last_granted_at, so on re-enrollment the grant timestamp won't be updated even when credits are successfully granted.
Compare with enrollContributorChampion which grants first (line 728), then persists the timestamp conditional on the result (lines 742/752). This function should follow the same pattern: insert/upsert the membership first without credits_last_granted_at, grant credits, then update the timestamp based on the outcome.
| enrolled_tier: resolvedTier, | ||
| enrolled_at: now, | ||
| credit_amount_microdollars: creditAmountMicrodollars, | ||
| credits_last_granted_at: creditGranted ? now : null, |
There was a problem hiding this comment.
[WARNING]: Re-enrollment unconditionally overwrites credits_last_granted_at to null when credits aren't re-granted
If a previously-enrolled contributor is re-enrolled at a $0 tier or without a linked Kilo user, creditGranted will be false and this sets credits_last_granted_at: null, erasing the historical timestamp of when credits were last granted. The cron's refreshContributorChampionCredits (line 794) uses this column to determine the 30-day cycle — setting it to null means credits could be re-granted prematurely on the next cron run if the contributor's tier is later upgraded.
Consider preserving the existing value when credits aren't newly granted, e.g.:
credits_last_granted_at: creditGranted ? now : sql`contributor_champion_memberships.credits_last_granted_at`,| if (row.allTimeContributions >= 15 && currentTier !== 'champion') { | ||
| newTier = 'champion'; | ||
| } else if ( | ||
| row.allTimeContributions >= 5 && |
There was a problem hiding this comment.
[WARNING]: Ambassador auto-upgrade threshold uses allTimeContributions instead of 90-day rolling window
getContributorTierSuggestion() (line 199) uses contributions90d >= 5 for ambassador, but this auto-upgrade check uses row.allTimeContributions >= 5. A contributor with 5 all-time contributions but 0 recent activity would be auto-upgraded to ambassador despite the suggestion engine not recommending them for that tier.
If this divergence is intentional per the implementation plan (which states "5+ merged PRs" without specifying a window), add a code comment documenting the difference from getContributorTierSuggestion.
| await insertEvent({ | ||
| contributorId: contributorBoundaryId, | ||
| prNumber: 2_000, | ||
| mergedAt: daysAgo(90), |
There was a problem hiding this comment.
[WARNING]: daysAgo(90) is an exact boundary — potential flaky test
The leaderboard SQL uses merged_at >= now() - interval '90 days'. Whether this event at exactly 90 days ago falls within the window depends on sub-second timing between the JS Date.now() call (line 42: daysAgo helper) and the Postgres now() evaluation in the query. If the DB evaluates now() even slightly later than the JS timestamp, the event will be excluded and the assertion at line 150 (contributions90d === 1) will fail intermittently.
| mergedAt: daysAgo(90), | |
| mergedAt: daysAgo(89), |
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
639c923 to
189b490
Compare
| SELECT DISTINCT e.contributor_id | ||
| FROM contributor_champion_events e | ||
| INNER JOIN kilocode_users u ON lower(u.google_user_email) = lower(e.github_author_email) | ||
| WHERE u.id = ${input.userId} |
There was a problem hiding this comment.
[WARNING]: Profile badge query only resolves contributors via event email matching — manually enrolled contributors (via manualEnrollContributor) who have no events with a matching email will never see a profile badge.
The matched_contributors CTE only checks contributor_champion_events.github_author_email against the user's email. It does not consider contributor_champion_memberships.linked_kilo_user_id, which is the explicit link set during manual enrollment.
Fix: add a UNION to also match by linked_kilo_user_id:
WITH matched_contributors AS (
SELECT DISTINCT e.contributor_id
FROM contributor_champion_events e
INNER JOIN kilocode_users u ON lower(u.google_user_email) = lower(e.github_author_email)
WHERE u.id = ${input.userId}
UNION
SELECT m.contributor_id
FROM contributor_champion_memberships m
WHERE m.linked_kilo_user_id = ${input.userId}
)| const syncResult = await syncContributorChampionData(); | ||
|
|
||
| // Step 2: Upgrade tiers based on merged PR counts | ||
| const upgradeResult = await processAutoTierUpgrades(); |
There was a problem hiding this comment.
[WARNING]: If syncContributorChampionData() (line 25) throws, both processAutoTierUpgrades() and refreshContributorChampionCredits() are skipped entirely because all three calls share a single try/catch.
Tier upgrades and credit refreshes don't depend on a successful sync — they operate on already-persisted data. A transient GitHub API failure during sync should not block credit grants (which could be 30+ days overdue).
Consider wrapping each step independently or using a Promise.allSettled-style pattern so partial failures don't block the remaining steps.
| const currentIndex = nextIndex; | ||
| nextIndex += 1; | ||
| if (currentIndex >= values.length) return; | ||
| const value = values[currentIndex]!; |
There was a problem hiding this comment.
[SUGGESTION]: Non-null assertion ! operator violates the project coding style ("STRONGLY AVOID typescript's null-forgiving !").
While safe here due to the bounds check on line 252, it can be replaced with a guard that satisfies TS flow analysis:
| const value = values[currentIndex]!; | |
| const value = values[currentIndex]; |
Since currentIndex < values.length is already guaranteed by the guard above, TypeScript should narrow this correctly. If not, use if (value === undefined) return; after the assignment.
| }>(sql` | ||
| SELECT id, google_user_email, google_user_name | ||
| FROM kilocode_users | ||
| WHERE lower(google_user_email) LIKE ${`%${query.toLowerCase()}%`} |
There was a problem hiding this comment.
[SUGGESTION]: SQL LIKE wildcards (%, _) in query are not escaped before interpolation into the LIKE pattern.
While this is parameterized (no SQL injection risk) and admin-only, an admin searching for % would match all rows, and _ would match any single character. This could return unexpected results.
Consider escaping wildcards:
const escaped = query.toLowerCase().replace(/[%_]/g, '\\$&');then use escaped in the LIKE pattern.
| const githubLogin = input.githubLogin ?? `manual-${input.email.split('@')[0]}`; | ||
| const githubProfileUrl = input.githubLogin | ||
| ? `https://github.com/${input.githubLogin}` | ||
| : ''; |
There was a problem hiding this comment.
[WARNING]: When githubLogin is null, github_profile_url is set to an empty string ''. This is stored in the DB and rendered as <a href=""> in the admin page, which navigates to the current page on click.
This column is text().notNull() in the schema, so null isn't an option without a migration. Consider using a placeholder like '#' or skipping the <a> tag when the URL is empty on the frontend side.
| : ''; | |
| : '#'; |
src/app/admin/contributors/page.tsx
Outdated
| disabled={ | ||
| manualEnrollMutation.isPending || | ||
| !manualEmail || | ||
| !manualEmail.includes('@') |
There was a problem hiding this comment.
[SUGGESTION]: manualEmail.includes('@') is an extremely weak email validation — the string @ alone passes it. The server-side router uses z.string().email(), so invalid emails will be rejected with a generic tRPC error instead of a clear client-side message.
Consider using a slightly stricter check to give better UX:
| !manualEmail.includes('@') | |
| !manualEmail.includes('@') || manualEmail.indexOf('@') < 1 |
Or better yet, use a regex like /^[^@]+@[^@]+\.[^@]+$/ to catch obvious bad inputs before the server round-trip.
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
189b490 to
2e37220
Compare
| for (const commit of commits) { | ||
| const commitEmail = commit.commit.author?.email; | ||
| if (commitEmail) { | ||
| return commitEmail.toLowerCase(); |
There was a problem hiding this comment.
[WARNING]: Fallback email resolution returns the first commit email from any committer, not just the PR author
If the author-matching loop (lines 349-356) finds no match, this fallback returns the email from the first commit in the PR regardless of authorship. For PRs with co-authored commits, merge commits, or bot-authored fixup commits, this would attribute the wrong email to the contributor — potentially linking them to the wrong Kilo account and granting credits to the wrong user.
Consider either removing this fallback entirely (return null instead) or adding a comment documenting why this trade-off is acceptable.
| c.id AS contributor_id, | ||
| c.github_login, | ||
| c.github_profile_url, | ||
| le.github_author_email AS email, |
There was a problem hiding this comment.
[WARNING]: Leaderboard query does not resolve manual_email for manually enrolled contributors
The last_email CTE only extracts emails from contributor_champion_events. Contributors created via manualEnrollContributor (which stores the email in contributor_champion_contributors.manual_email) who have no events will show null for email here. This cascades:
- The
kilocode_usersJOIN on line 587 won't match, solinked_user_id/linked_user_namewill be null - The admin UI will show the "No Kilo account" warning icon even though
linked_kilo_user_idis set on the membership - The enrolled list will appear to have unlinked contributors that are actually linked
Consider adding a COALESCE(le.github_author_email, c.manual_email) or a second CTE that falls back to manual_email.
|
|
||
| const creditAmountUsd = TIER_CREDIT_USD[resolvedTier]; | ||
| const creditAmountMicrodollars = toMicrodollars(creditAmountUsd); | ||
| const linkedKiloUserId = row.linkedUserId; |
There was a problem hiding this comment.
[WARNING]: enrollContributorChampion resolves linkedKiloUserId from row.linkedUserId (email-based match) instead of row.linkedKiloUserId (membership-based)
For manually enrolled contributors who have linked_kilo_user_id set on their membership but no event email match, row.linkedUserId will be null while row.linkedKiloUserId would have the correct value. This means re-enrolling a manually enrolled contributor would lose their Kilo user link and fail to grant credits.
Consider: const linkedKiloUserId = row.linkedKiloUserId ?? row.linkedUserId;
src/app/admin/contributors/page.tsx
Outdated
| disabled={ | ||
| manualEnrollMutation.isPending || | ||
| !manualEmail || | ||
| !manualEmail.includes('@') |
There was a problem hiding this comment.
[SUGGESTION]: Email validation using !manualEmail.includes('@') is weak — emails like @ or foo@ would pass
The tRPC router uses z.string().email() (line 97 of the router), which will catch these server-side. But the button would be enabled, the request would fire, and then fail with a tRPC error. Consider matching the validation here to reduce unnecessary round-trips, e.g. using a simple regex like /^[^@]+@[^@]+\.[^@]+$/.
| const creditAmountMicrodollars = toMicrodollars(creditAmountUsd); | ||
|
|
||
| // Create or find contributor record | ||
| const githubLogin = input.githubLogin ?? `manual-${input.email.split('@')[0]}`; |
There was a problem hiding this comment.
[WARNING]: Synthesized github_login for manual enrollees can collide with real GitHub users
When githubLogin is null, the fallback manual-${input.email.split('@')[0]} could match an existing contributor's github_login if someone later creates a GitHub account with that name, or if two manual enrollees share the same email prefix. Since github_login has a UNIQUE constraint, the onConflictDoUpdate at line 1005 would overwrite the existing contributor's manual_email.
Consider using a more collision-resistant prefix like manual-enroll-${uuid()} or at minimum adding the full email hash.
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
2e37220 to
8dfd5d8
Compare
| c.id AS contributor_id, | ||
| c.github_login, | ||
| c.github_profile_url, | ||
| le.github_author_email AS email, |
There was a problem hiding this comment.
[WARNING]: Leaderboard query resolves email only from events (last_email CTE), ignoring contributor_champion_contributors.manual_email
Manually enrolled contributors who have no PR events will show null for both email and linked_user_id in the leaderboard/enrolled list. The admin UI's EmailCell component will display "—" with a "No Kilo account" tooltip, even when the membership has a valid linked_kilo_user_id.
Consider using COALESCE(le.github_author_email, c.manual_email) as the email source, and also joining kilocode_users on m.linked_kilo_user_id as a fallback for the user link. Otherwise, manually enrolled contributors will appear broken in the enrolled table.
| const creditAmountMicrodollars = toMicrodollars(creditAmountUsd); | ||
|
|
||
| // Create or find contributor record | ||
| const githubLogin = input.githubLogin ?? `manual-${input.email.split('@')[0]}`; |
There was a problem hiding this comment.
[WARNING]: Generated github_login from email prefix risks collisions between unrelated manual enrollments
If two contributors are manually enrolled with different email addresses that share the same local part (e.g., john@gmail.com and john@company.com), both resolve to manual-john. The second onConflictDoUpdate at line 1010 would overwrite the first contributor's manual_email, silently merging two distinct people into one record.
Consider appending a UUID suffix or using the full email as part of the generated login (e.g., manual-john-at-company-com) to avoid collisions.
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
8dfd5d8 to
db71a2f
Compare
| .limit(1); | ||
|
|
||
| if (linkedUser) { | ||
| creditGranted = await grantContributorChampionCredits(linkedUser, creditAmountUsd); |
There was a problem hiding this comment.
[WARNING]: TOCTOU race — concurrent enrollments can double-grant credits
Credits are granted here (line 731) before the membership upsert at line 737. If two concurrent admin requests enroll the same contributor simultaneously:
- Both call
getContributorChampionLeaderboard()and see noenrolled_tier - Both call
grantContributorChampionCredits()→ credits granted twice - Both upsert the membership → last write wins, but credits are already double-granted
The onConflictDoUpdate on the membership prevents duplicate rows but does not prevent duplicate credit grants because the grant happens outside the upsert.
Consider wrapping the credit grant + membership upsert in a serializable transaction with a SELECT ... FOR UPDATE on the contributor row, or restructure to insert-then-grant so you can check the upsert result (inserted vs updated) before granting.
| } | ||
|
|
||
| const amountUsd = fromMicrodollars(Number(membership.credit_amount_microdollars)); | ||
| const granted = await grantContributorChampionCredits(linkedUser, amountUsd); |
There was a problem hiding this comment.
[WARNING]: TOCTOU race — concurrent cron runs can double-grant monthly credits
Eligible rows are fetched at lines 786–801, then processed sequentially. credits_last_granted_at is only updated at lines 827–833 after each grant succeeds. If two cron invocations overlap (e.g., during deployments or queue retries), both read the same eligible rows and both grant credits before either updates the timestamp.
This is particularly concerning since this is a financial operation (granting real credit to user accounts).
Consider adding a SELECT ... FOR UPDATE SKIP LOCKED pattern or a distributed lock (e.g., advisory lock) to serialize credit grants per membership.
| }; | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: 'BAD_REQUEST', |
There was a problem hiding this comment.
[WARNING]: All errors classified as BAD_REQUEST, masking internal failures
This catch block maps every error — including DB connection failures, network timeouts, and unexpected exceptions — to BAD_REQUEST. This:
- Makes it harder to distinguish client errors from server bugs in monitoring/alerting
- Loses the original stack trace (only
error.messageis propagated) - Tells the client it did something wrong when the server may have failed
Consider either letting unexpected errors propagate as INTERNAL_SERVER_ERROR (tRPC's default), or catching only expected validation errors explicitly. The same issue exists in the manualEnroll catch block at line 112.
| <TableCell> | ||
| <div className="flex flex-col gap-1"> | ||
| <Select | ||
| value={effectiveTier ?? undefined} |
There was a problem hiding this comment.
[WARNING]: effectiveTier ?? undefined causes controlled → uncontrolled switch
When effectiveTier is null, this evaluates to undefined, making the <Select> uncontrolled. Once a tier is selected, it becomes controlled with a string value. React warns against switching between controlled and uncontrolled components.
Use a sentinel value instead:
| value={effectiveTier ?? undefined} | |
| value={effectiveTier ?? '__none__'} |
Then handle '__none__' in the onValueChange handler, or add a placeholder <SelectItem value="__none__">Select tier…</SelectItem> to the options.
| html_url: 'https://github.com/Kilo-Org/kilocode/pull/7103', | ||
| title: 'External contribution 2', | ||
| pull_request: { | ||
| merged_at: '2026-03-01T00:00:00.000Z', |
There was a problem hiding this comment.
[WARNING]: Hard-coded absolute date will make this test flaky after May 30, 2026
merged_at: '2026-03-01T00:00:00.000Z' is an absolute timestamp. The leaderboard query's 90-day window uses now() - interval '90 days', so after May 30, 2026 this date will fall outside the window. The assertion at line 312 (contributions90d === 2) will then fail.
The same issue affects '2026-02-01T00:00:00.000Z' at lines 214 and 228 (will fail after May 2, 2026).
Use relative dates instead, e.g.:
merged_at: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(), // 7 days ago
- Admin page: pagination (10 rows/page), sortable columns, filter dropdowns - Enrolled section moved above Review Queue - Email column links to Kilo profile, tooltip for unlinked accounts - Team exclusion for kilocode.ai, kilo.ai domains and vincesprints@gmail.com - Contributor champion credits: Ambassador 0/mo, Champion 50/mo - Auto-tier upgrades (5 PRs -> Ambassador, 15 PRs -> Champion) - 30-day rolling credit grants with expiry - Cron job for auto-upgrades and credit refresh - DB migration 0047 for credit tracking columns - Promo category 'contributor-champion-credits' registered
bturcotte520
force-pushed
the
brian/addContributorDash
branch
from
db71a2f to
2ff7ea7
Compare
1 participant
Summary
Verification
Visual Changes
Reviewer Notes