fix: auto-patch for issue #265 - #266
Conversation
|
@Crazy-Monkey is attempting to deploy a commit to the Vezures Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughChangesBounty winner selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant selectBountyWinner
participant BountyData
participant PostStats
participant UserProfiles
participant AuditLog
Caller->>selectBountyWinner: bountyId
selectBountyWinner->>BountyData: load bounty and active entries
BountyData-->>selectBountyWinner: candidates in time window
selectBountyWinner->>PostStats: aggregate engagement metrics
PostStats-->>selectBountyWinner: scores
selectBountyWinner->>UserProfiles: check account age and email verification
UserProfiles-->>selectBountyWinner: eligibility data
selectBountyWinner->>AuditLog: persist selection audit
selectBountyWinner-->>Caller: SelectionResult
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)sdk/plugin-tinyplace/src/bounty/winner.tsFile contains syntax errors that prevent linting: Line 90: expected 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f798097db6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| import { getDb } from '../db'; | ||
| import { Bounty, BountyEntry, BountyWinner } from '../models'; | ||
| import { v4 as uuid } from 'uuid'; |
There was a problem hiding this comment.
When this new module is imported, it fails during module resolution before selectBountyWinner can run: ../db and ../models do not exist under sdk/plugin-tinyplace/src (checked with rg --files sdk/plugin-tinyplace/src), and uuid is not declared in sdk/plugin-tinyplace/package.json. Any caller trying to use the bounty winner selector from the published plugin will hit a load-time error, so this needs to use existing plugin/SDK modules or add the missing files and dependency.
Useful? React with 👍 / 👎.
| })); | ||
|
|
||
| // Apply eligibility filters (example: account age >= 7 days, email verified) | ||
| const eligible = scores.filter(async (s) => { |
There was a problem hiding this comment.
When any entry belongs to a missing, too-new, or unverified user, this filter callback returns a Promise object, and Array.prototype.filter treats every Promise as truthy instead of waiting for the boolean result. That means finalScores still contains every entry, so an ineligible account can be sorted to the top and win despite the audit rationale saying the filters were applied; compute the async eligibility results with Promise.all(scores.map(...)) and then filter on the resolved booleans.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/plugin-tinyplace/src/bounty/winner.ts`:
- Around line 20-89: Refactor selectBountyWinner into a concise orchestration
function under 20 lines by extracting score aggregation into a computeScores
helper and asynchronous eligibility evaluation into a filterEligibleEntries
helper. Wrap the main database and winner-selection flow in try-catch, handling
unexpected runtime or database errors according to the project’s established
error-handling pattern while preserving the current scoring, eligibility, audit
logging, and result behavior.
- Around line 45-63: Replace the async filter in the winner eligibility flow
with a Promise-based map that returns each score when eligible and null
otherwise, then await the mapped results and synchronously remove null values.
Update callback variables such as `s` and `v` to descriptive names while
preserving the existing account-age and email-verification checks.
- Around line 69-70: Ensure deterministic winner ordering by adding a secondary
tie-breaker to the comparator used for sorting finalScores in the
winner-selection flow. Preserve descending score order, and when scores are
equal compare the stable entry identifier (such as entryId) consistently; update
the related entries query ordering if needed so that the identifier is available
and deterministic.
- Around line 1-3: Add uuid as a direct dependency in
sdk/plugin-tinyplace/package.json, then clean up the imports in winner.ts: place
the external uuid import before local imports, import Bounty and BountyEntry as
types, and remove the unused BountyWinner import.
🪄 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: CHILL
Plan: Pro
Run ID: aba09827-b6eb-4d67-8136-e829f508549b
📒 Files selected for processing (1)
sdk/plugin-tinyplace/src/bounty/winner.ts
| import { getDb } from '../db'; | ||
| import { Bounty, BountyEntry, BountyWinner } from '../models'; | ||
| import { v4 as uuid } from 'uuid'; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if uuid is listed in package.json dependencies
cat sdk/plugin-tinyplace/package.json | jq '.dependencies.uuid, .devDependencies.uuid'Repository: tinyhumansai/tiny.place
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== winner.ts (outline) =="
ast-grep outline sdk/plugin-tinyplace/src/bounty/winner.ts --view expanded || true
echo
echo "== winner.ts (full with line numbers, capped) =="
wc -l sdk/plugin-tinyplace/src/bounty/winner.ts
cat -n sdk/plugin-tinyplace/src/bounty/winner.ts | sed -n '1,220p'
echo
echo "== package.json uuid refs =="
cat sdk/plugin-tinyplace/package.json | jq '.dependencies.uuid, .devDependencies.uuid'
echo
echo "== related model exports =="
ast-grep outline sdk/plugin-tinyplace/src/models.ts --view expanded || true
ast-grep outline sdk/plugin-tinyplace/src/models/index.ts --view expanded || true
echo
echo "== db file outline =="
ast-grep outline sdk/plugin-tinyplace/src/db.ts --view expanded || true
ast-grep outline sdk/plugin-tinyplace/src/db/index.ts --view expanded || trueRepository: tinyhumansai/tiny.place
Length of output: 5688
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect the file and the referenced modules if the broader outline is available.
for f in \
sdk/plugin-tinyplace/src/bounty/winner.ts \
sdk/plugin-tinyplace/src/models.ts \
sdk/plugin-tinyplace/src/models/index.ts \
sdk/plugin-tinyplace/src/db.ts \
sdk/plugin-tinyplace/src/db/index.ts
do
if [ -f "$f" ]; then
echo "== $f =="
wc -l "$f"
fi
doneRepository: tinyhumansai/tiny.place
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=sdk/plugin-tinyplace/src/bounty/winner.ts
echo "== $file =="
wc -l "$file"
cat -n "$file" | sed -n '1,220p'Repository: tinyhumansai/tiny.place
Length of output: 4845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package manifests mentioning uuid =="
rg -n --glob 'package.json' '"uuid"|\buuid\b' . || true
echo
echo "== workspace/package roots =="
git ls-files '**/package.json'Repository: tinyhumansai/tiny.place
Length of output: 409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether uuid is declared at the repo root or only missing in the plugin package.
for f in package.json sdk/plugin-tinyplace/package.json; do
if [ -f "$f" ]; then
echo "== $f =="
jq '.dependencies.uuid, .devDependencies.uuid, .peerDependencies.uuid' "$f" 2>/dev/null || cat "$f"
fi
doneRepository: tinyhumansai/tiny.place
Length of output: 250
Add uuid as a direct dependency and clean up the import block.
uuid isn’t declared in sdk/plugin-tinyplace/package.json, so a clean install won’t resolve this import. Move the external import above the local ones, switch Bounty/BountyEntry to import type, and drop unused BountyWinner.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/plugin-tinyplace/src/bounty/winner.ts` around lines 1 - 3, Add uuid as a
direct dependency in sdk/plugin-tinyplace/package.json, then clean up the
imports in winner.ts: place the external uuid import before local imports,
import Bounty and BountyEntry as types, and remove the unused BountyWinner
import.
Source: Coding guidelines
| export async function selectBountyWinner(bountyId: string): Promise<SelectionResult> { | ||
| const db = getDb(); | ||
| const bounty = await db.get<Bounty>('SELECT * FROM bounties WHERE id = ?', [bountyId]); | ||
| if (!bounty) throw new Error(`Bounty ${bountyId} not found`); | ||
|
|
||
| const now = Date.now(); | ||
| const entries = await db.all<BountyEntry[]>( | ||
| `SELECT * FROM bounty_entries WHERE bounty_id = ? AND status = 'active' AND created_at >= ? AND created_at <= ?`, | ||
| [bountyId, bounty.startTime, bounty.endTime] | ||
| ); | ||
|
|
||
| // Compute scores: impressions + comments + engagement | ||
| const scores = await Promise.all(entries.map(async (entry) => { | ||
| const stats = await db.get<{ impressions: number; comments: number; engagement: number }>( | ||
| `SELECT COALESCE(SUM(impressions),0) AS impressions, | ||
| COALESCE(SUM(comments),0) AS comments, | ||
| COALESCE(SUM(engagement),0) AS engagement | ||
| FROM post_stats WHERE user_id = ? AND post_time >= ? AND post_time <= ?`, | ||
| [entry.userId, bounty.startTime, bounty.endTime] | ||
| ); | ||
| const totalScore = stats.impressions + stats.comments + stats.engagement; | ||
| return { entryId: entry.id, userId: entry.userId, score: totalScore }; | ||
| })); | ||
|
|
||
| // Apply eligibility filters (example: account age >= 7 days, email verified) | ||
| const eligible = scores.filter(async (s) => { | ||
| const user = await db.get<{ createdAt: number; emailVerified: boolean }>( | ||
| 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?', | ||
| [s.userId] | ||
| ); | ||
| if (!user) return false; | ||
| const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24); | ||
| if (accountAgeDays < 7) { | ||
| console.warn(`User ${s.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log | ||
| return false; | ||
| } | ||
| if (!user.emailVerified) { | ||
| console.warn(`User ${s.userId} disqualified: email not verified`); | ||
| return false; | ||
| } | ||
| return true; | ||
| }); | ||
| const eligibleScores = await Promise.all(eligible); // resolve async filter | ||
| const finalScores = eligibleScores.filter(Boolean).map((v) => v!); | ||
|
|
||
| if (finalScores.length === 0) { | ||
| throw new Error('No eligible entries for bounty'); | ||
| } | ||
|
|
||
| // Sort descending by score | ||
| finalScores.sort((a, b) => b.score - a.score); | ||
| const winner = entries.find(e => e.id === finalScores[0].entryId)!; | ||
| const runnerUp = finalScores.length > 1 ? entries.find(e => e.id === finalScores[1].entryId) : null; | ||
|
|
||
| const result: SelectionResult = { | ||
| winner, | ||
| runnerUp: runnerUp ?? null, | ||
| scores: finalScores.map(s => ({ entryId: s.entryId, score: s.score })), | ||
| queryTimestamp: now, | ||
| rationale: `Total engagement score (impressions+comments+engagement) computed from post_stats for period [${bounty.startTime}, ${bounty.endTime}]. Eligibility filters applied: account age >=7 days, email verified. Winner: user ${winner.userId} with score ${finalScores[0].score}.` | ||
| }; | ||
|
|
||
| // Log audit trail to database | ||
| await db.run( | ||
| `INSERT INTO bounty_audit_log (id, bounty_id, winner_user_id, winner_score, runner_up_user_id, runner_up_score, query_timestamp, rationale, scores_json) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, | ||
| [uuid(), bountyId, winner.userId, finalScores[0].score, runnerUp?.userId ?? null, finalScores[1]?.score ?? null, now, result.rationale, JSON.stringify(result.scores)] | ||
| ); | ||
|
|
||
| return result; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Wrap main async function in try-catch and extract helper functions.
The selectBountyWinner function violates two project coding guidelines:
- It lacks a
try-catchblock, which is required for all async functions to handle unexpected runtime or database errors gracefully. - It exceeds the 20-line length limit.
Please extract the scoring aggregation and the eligibility checks into separate helper functions (e.g., computeScores, filterEligibleEntries) and wrap the main execution flow within a try-catch block.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/plugin-tinyplace/src/bounty/winner.ts` around lines 20 - 89, Refactor
selectBountyWinner into a concise orchestration function under 20 lines by
extracting score aggregation into a computeScores helper and asynchronous
eligibility evaluation into a filterEligibleEntries helper. Wrap the main
database and winner-selection flow in try-catch, handling unexpected runtime or
database errors according to the project’s established error-handling pattern
while preserving the current scoring, eligibility, audit logging, and result
behavior.
Source: Coding guidelines
| const eligible = scores.filter(async (s) => { | ||
| const user = await db.get<{ createdAt: number; emailVerified: boolean }>( | ||
| 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?', | ||
| [s.userId] | ||
| ); | ||
| if (!user) return false; | ||
| const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24); | ||
| if (accountAgeDays < 7) { | ||
| console.warn(`User ${s.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log | ||
| return false; | ||
| } | ||
| if (!user.emailVerified) { | ||
| console.warn(`User ${s.userId} disqualified: email not verified`); | ||
| return false; | ||
| } | ||
| return true; | ||
| }); | ||
| const eligibleScores = await Promise.all(eligible); // resolve async filter | ||
| const finalScores = eligibleScores.filter(Boolean).map((v) => v!); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix critical logic flaw in async filter and avoid abbreviations.
Array.prototype.filter expects a synchronous callback. Passing an async function returns a Promise, which evaluates to true. This entirely bypasses the eligibility checks, allowing users with unverified emails or new accounts to win.
To resolve this, map the items to promises that return the item if eligible (or null otherwise), await them, and then synchronously filter out the null values. Additionally, avoid abbreviations like s and v as per coding guidelines.
🐛 Proposed fix for the eligibility filter
- const eligible = scores.filter(async (s) => {
- const user = await db.get<{ createdAt: number; emailVerified: boolean }>(
- 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?',
- [s.userId]
- );
- if (!user) return false;
- const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24);
- if (accountAgeDays < 7) {
- console.warn(`User ${s.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log
- return false;
- }
- if (!user.emailVerified) {
- console.warn(`User ${s.userId} disqualified: email not verified`);
- return false;
- }
- return true;
- });
- const eligibleScores = await Promise.all(eligible); // resolve async filter
- const finalScores = eligibleScores.filter(Boolean).map((v) => v!);
+ const finalScores = (
+ await Promise.all(
+ scores.map(async (scoreItem) => {
+ const user = await db.get<{ createdAt: number; emailVerified: boolean }>(
+ 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?',
+ [scoreItem.userId]
+ );
+ if (!user) return null;
+ const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24);
+ if (accountAgeDays < 7) {
+ console.warn(`User ${scoreItem.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log
+ return null;
+ }
+ if (!user.emailVerified) {
+ console.warn(`User ${scoreItem.userId} disqualified: email not verified`);
+ return null;
+ }
+ return scoreItem;
+ })
+ )
+ ).filter((scoreItem): scoreItem is NonNullable<typeof scoreItem> => scoreItem !== null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const eligible = scores.filter(async (s) => { | |
| const user = await db.get<{ createdAt: number; emailVerified: boolean }>( | |
| 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?', | |
| [s.userId] | |
| ); | |
| if (!user) return false; | |
| const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24); | |
| if (accountAgeDays < 7) { | |
| console.warn(`User ${s.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log | |
| return false; | |
| } | |
| if (!user.emailVerified) { | |
| console.warn(`User ${s.userId} disqualified: email not verified`); | |
| return false; | |
| } | |
| return true; | |
| }); | |
| const eligibleScores = await Promise.all(eligible); // resolve async filter | |
| const finalScores = eligibleScores.filter(Boolean).map((v) => v!); | |
| const finalScores = ( | |
| await Promise.all( | |
| scores.map(async (scoreItem) => { | |
| const user = await db.get<{ createdAt: number; emailVerified: boolean }>( | |
| 'SELECT created_at AS createdAt, email_verified AS emailVerified FROM users WHERE id = ?', | |
| [scoreItem.userId] | |
| ); | |
| if (!user) return null; | |
| const accountAgeDays = (now - user.createdAt) / (1000 * 60 * 60 * 24); | |
| if (accountAgeDays < 7) { | |
| console.warn(`User ${scoreItem.userId} disqualified: account age ${accountAgeDays.toFixed(1)} days < 7`); // audit log | |
| return null; | |
| } | |
| if (!user.emailVerified) { | |
| console.warn(`User ${scoreItem.userId} disqualified: email not verified`); | |
| return null; | |
| } | |
| return scoreItem; | |
| }) | |
| ) | |
| ).filter((scoreItem): scoreItem is NonNullable<typeof scoreItem> => scoreItem !== null); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/plugin-tinyplace/src/bounty/winner.ts` around lines 45 - 63, Replace the
async filter in the winner eligibility flow with a Promise-based map that
returns each score when eligible and null otherwise, then await the mapped
results and synchronously remove null values. Update callback variables such as
`s` and `v` to descriptive names while preserving the existing account-age and
email-verification checks.
Source: Coding guidelines
| // Sort descending by score | ||
| finalScores.sort((a, b) => b.score - a.score); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Ensure deterministic sorting for tied scores.
The PR objectives explicitly require winner selection to be deterministic. If two entries achieve the exact same score, Array.prototype.sort() guarantees stability by falling back to their original order. However, the original database query for entries (Line 27) lacks an ORDER BY clause, meaning the base order is non-deterministic.
Introduce a secondary criteria (e.g., entryId) to guarantee a deterministic tie-breaker.
⚖️ Proposed fix
- // Sort descending by score
- finalScores.sort((a, b) => b.score - a.score);
+ // Sort descending by score, then ascending by entryId for determinism
+ finalScores.sort((scoreA, scoreB) => {
+ if (scoreB.score !== scoreA.score) return scoreB.score - scoreA.score;
+ return scoreA.entryId.localeCompare(scoreB.entryId);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Sort descending by score | |
| finalScores.sort((a, b) => b.score - a.score); | |
| // Sort descending by score, then ascending by entryId for determinism | |
| finalScores.sort((scoreA, scoreB) => { | |
| if (scoreB.score !== scoreA.score) return scoreB.score - scoreA.score; | |
| return scoreA.entryId.localeCompare(scoreB.entryId); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/plugin-tinyplace/src/bounty/winner.ts` around lines 69 - 70, Ensure
deterministic winner ordering by adding a secondary tie-breaker to the
comparator used for sorting finalScores in the winner-selection flow. Preserve
descending score order, and when scores are equal compare the stable entry
identifier (such as entryId) consistently; update the related entries query
ordering if needed so that the identifier is available and deterministic.
2 participants
🤖 Auto-generated fix by Crazy Monkey
Closes #265
Generated automatically. Please review before merging.
Summary by CodeRabbit