Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @Await-0x, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a crucial migration of the client and indexer components to align with a new deployment of the Summit game contract. The changes ensure that all parts of the application correctly reference the updated contract address and synchronize data from the appropriate blockchain state, alongside minor adjustments to client-side data fetching and controller configuration to maintain compatibility and functionality with the new contract version. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated Summit Game contract address and related start block; adjusted Starknet controller options; removed a token filter from a DAO query; simplified MainPage by removing the MIGRATING flow and adding QuestBoard usage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
|
Claude finished @Await-0x's task —— View job Claude Review - General Engineering Reviewrun=22314405918 attempt=1 sha=f9e10951b04fb286880672545841acd3d7068136 scope=general Todo List
AnalysisI reviewed the changes in scope for this PR, which focuses on files outside the core app packages ( Change Summary:
Review FindingsNo issues found. The change is a straightforward documentation update that reflects the new Summit Game contract deployment. This is a pure documentation change with no behavioral impact on CI, workflows, automation, or developer tooling. The Voyager.online link is properly updated to match the new contract address, ensuring documentation accuracy. Summary: 0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
Codex Review - General Engineering ReviewNo issues found. |
|
Claude finished @Await-0x's task —— View job Claude Review - Indexer/API Reviewrun=22314405918 attempt=1 sha=f9e10951b04fb286880672545841acd3d7068136 scope=indexer-api No issues found. AnalysisOnly one file within the indexer/API scope was changed:
7-Area Review Framework Applied:1. Data Correctness and Idempotency: ✅ The indexer logic in 2. Filters, Selectors, and Stream Efficiency: ✅ The contract address is correctly used as a filter in the event stream configuration (line 936). The address format is properly normalized with 3. Event Decoding and Serde Safety: ✅ No decoder logic changes. The existing decoder functions remain valid for the new contract deployment. 4. Finality, Reorgs, and Indexer State: ✅ Starting block update from 6,866,000 to 7,077,225 (+211,225 blocks) is appropriate for a new contract deployment. The indexer maintains 5. Schema, Constraints, and Migrations: ✅ No schema changes required. Configuration-only changes don't affect database structure. 6. Reliability, Operations, and Config: ✅ Contract address and starting block are properly externalized in configuration. No hardcoded values or hidden dependencies. 7. Testing and Verification: ✅ Configuration changes are deterministic and easily verifiable against blockchain state. Migration Impact AssessmentThis appears to be a deployment migration from an old Summit contract to a new one. The changes are consistent:
Summary: 0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the client to a new summit contract address. The changes mostly involve updating contract addresses across the codebase, which appears to be done correctly. However, I've identified a significant issue in client/src/dojo/useGameTokens.ts where a filtering clause has been removed from a query, but the logic to generate that clause remains. This has resulted in dead code and a potential performance regression that should be addressed.
| WHERE o.owner = "${addAddressPadding(owner.toLowerCase())}" | ||
| AND mr.minter_address = "${addAddressPadding(currentNetworkConfig.dungeon)}" | ||
| AND tm.game_over = 1 | ||
| ${greaterThanClause} | ||
| ` |
There was a problem hiding this comment.
The ${greaterThanClause} has been removed from this query. However, the logic to calculate this variable (lines 25-39) still exists, making it dead code. This includes an API call to fetch claimedAdventurerIds which are now unused.
This has two implications:
- Maintainability: The unused code should be removed to keep the codebase clean and prevent confusion.
- Performance: Removing this filter might cause the function to fetch and process more data than necessary, potentially leading to performance degradation by re-fetching adventurers that have already been claimed.
If this filtering is no longer necessary, please remove the dead code. If it is still required, the clause should be re-added.
Codex Review - Indexer/API Review[HIGH] [MEDIUM] Summary: 0 CRITICAL, 1 HIGH, 1 MEDIUM, 0 LOW, 0 INFO |
![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: f9e10951b0
ℹ️ 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".
| AND tm.game_over = 1 | ||
| ${greaterThanClause} | ||
| ` |
There was a problem hiding this comment.
The Torii SQL no longer appends greaterThanClause, so getValidAdventurers now includes adventurer IDs that the /adventurers/:player API already returned as claimed. That makes adventurerCollection and corpse totals include already-claimed entries, and those IDs are then sent again in claim_corpse_reward batches from ClaimRewardsButton, which can prevent users from claiming remaining valid corpses once a batch contains previously claimed IDs; this is also inconsistent with the existing contract of useGameTokens captured by client/src/dojo/useGameTokens.test.tsx (it expects AND tm.id > ... to be present).
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 the current code and only fix it if needed.
Inline comments:
In `@client/.env`:
- Line 2: The file client/.env is missing a trailing newline which causes
dotenv-linter to fail; open the .env containing VITE_PUBLIC_SUMMIT_ADDRESS and
add a single blank line (newline character) at EOF so the file ends with a
newline character, then save to satisfy POSIX and the linter.
- Line 2: Replace the incorrect Summit contract address value for
VITE_PUBLIC_SUMMIT_ADDRESS with the canonical mainnet address
0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa in the
environment and update the mirrored hardcoded value in apibara.config.ts so both
use the same mainnet address; leave client/src/utils/networkConfig.ts as-is
since it already reads VITE_PUBLIC_SUMMIT_ADDRESS at runtime and verify the env
var name VITE_PUBLIC_SUMMIT_ADDRESS is unchanged.
In `@indexer/apibara.config.ts`:
- Line 7: Update the incorrect Summit contract address by replacing the value
assigned to summitContractAddress (in apibara config) and the
VITE_PUBLIC_SUMMIT_ADDRESS entry in the client env with the correct mainnet
address 0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa; also
verify that the configured startingBlock (startingBlock) corresponds to the
block where that Summit contract was deployed and, if it does not, change
startingBlock to the documented reference 6767900 (or the actual deployment
block) so both indexer and client use the correct address and aligned start
block.
In `@README.md`:
- Line 83: Replace the outdated Summit mainnet contract address in AGENTS.md
(currently 0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa at
line ~104) with the new address
0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc to match
README.md, and make the identical update in client/README.md (around line ~48)
so both documents reference the same canonical Summit contract address.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.mdclient/.envclient/src/contexts/starknet.tsxclient/src/dojo/useGameTokens.tsindexer/apibara.config.ts
💤 Files with no reviewable changes (1)
- client/src/dojo/useGameTokens.ts
| @@ -1,2 +1,2 @@ | |||
| VITE_PUBLIC_CHAIN=SN_MAIN | |||
| VITE_PUBLIC_SUMMIT_ADDRESS=0x0214d382e80781f8c1059a751563d6b46e717c652bb670bf230e8a64a68e6064 No newline at end of file | |||
| VITE_PUBLIC_SUMMIT_ADDRESS=0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc No newline at end of file | |||
There was a problem hiding this comment.
Missing blank line at end of file.
dotenv-linter flags a missing newline at EOF. Add a trailing newline to comply with POSIX and the linter.
🔧 Proposed fix
VITE_PUBLIC_CHAIN=SN_MAIN
VITE_PUBLIC_SUMMIT_ADDRESS=0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc
+📝 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.
| VITE_PUBLIC_SUMMIT_ADDRESS=0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc | |
| VITE_PUBLIC_SUMMIT_ADDRESS=0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/.env` at line 2, The file client/.env is missing a trailing newline
which causes dotenv-linter to fail; open the .env containing
VITE_PUBLIC_SUMMIT_ADDRESS and add a single blank line (newline character) at
EOF so the file ends with a newline character, then save to satisfy POSIX and
the linter.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current state of networkConfig.ts
echo "=== Content of networkConfig.ts ==="
cat client/src/utils/networkConfig.ts
echo -e "\n=== Check for Summit address references in codebase ==="
rg -n "0x0214d382e80781f8c1059a751563d6b46e717c652bb670bf230e8a64a68e6064|0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa|0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc" --type ts --type txt -A 2 -B 2Repository: Provable-Games/summit
Length of output: 11888
The Summit address in this PR does not match the mainnet guideline.
The .env file uses 0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc, but the coding guidelines specify the mainnet Summit address should be 0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa. The indexer/apibara.config.ts mirrors this incorrect address. While client/src/utils/networkConfig.ts correctly reads the address from the environment at runtime, the environment itself points to the wrong contract for mainnet deployment.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/.env` at line 2, Replace the incorrect Summit contract address value
for VITE_PUBLIC_SUMMIT_ADDRESS with the canonical mainnet address
0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa in the
environment and update the mirrored hardcoded value in apibara.config.ts so both
use the same mainnet address; leave client/src/utils/networkConfig.ts as-is
since it already reads VITE_PUBLIC_SUMMIT_ADDRESS at runtime and verify the env
var name VITE_PUBLIC_SUMMIT_ADDRESS is unchanged.
| summit: { | ||
| // Summit game contract address (mainnet) | ||
| summitContractAddress: "0x0214d382e80781f8c1059a751563d6b46e717c652bb670bf230e8a64a68e6064", | ||
| summitContractAddress: "0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n indexer/apibara.config.ts | head -50Repository: Provable-Games/summit
Length of output: 2330
🏁 Script executed:
rg "0x01aa95ea|0x0455c737" --type tsRepository: Provable-Games/summit
Length of output: 423
🏁 Script executed:
rg "summitContractAddress|Summit.*0x" client/src/utils/networkConfig.ts client/.env --type ts --type txt -A 1Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
rg "0x0455c737|0x01aa95ea" client/ -t ts -t txt -t envRepository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
cat client/.env | grep -i summitRepository: Provable-Games/summit
Length of output: 158
🏁 Script executed:
cat client/src/utils/networkConfig.ts | grep -i "0x0455c737\|0x01aa95ea\|summitContractAddress" -A 2 -B 2Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
head -100 client/src/utils/networkConfig.ts | grep -i "0x\|summit" -A 1 -B 1Repository: Provable-Games/summit
Length of output: 1219
🏁 Script executed:
git log --oneline --all -- client/src/utils/networkConfig.ts indexer/apibara.config.ts client/.env | head -20Repository: Provable-Games/summit
Length of output: 605
Incorrect Summit contract address across indexer and client configuration.
The summitContractAddress in indexer/apibara.config.ts line 7 is 0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc, but the project guidelines require the mainnet address 0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa. This same incorrect address is also present in client/.env as VITE_PUBLIC_SUMMIT_ADDRESS. Update both files to use the correct mainnet address.
Additionally, the startingBlock at line 28 is 7077225, which deviates from the documented reference value of 6767900. Confirm this block number aligns with the deployment of the correct Summit contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@indexer/apibara.config.ts` at line 7, Update the incorrect Summit contract
address by replacing the value assigned to summitContractAddress (in apibara
config) and the VITE_PUBLIC_SUMMIT_ADDRESS entry in the client env with the
correct mainnet address
0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa; also verify
that the configured startingBlock (startingBlock) corresponds to the block where
that Summit contract was deployed and, if it does not, change startingBlock to
the documented reference 6767900 (or the actual deployment block) so both
indexer and client use the correct address and aligned start block.
| | Contract | Address | | ||
| | ----------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | Summit Game | [`0x0214d382e80781f8c1059a751563d6b46e717c652bb670bf230e8a64a68e6064`](https://voyager.online/contract/0x0214d382e80781f8c1059a751563d6b46e717c652bb670bf230e8a64a68e6064) | | ||
| | Summit Game | [`0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc`](https://voyager.online/contract/0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc) | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find AGENTS.md file
fd -t f "AGENTS\.md" .Repository: Provable-Games/summit
Length of output: 153
🏁 Script executed:
# Search for old and new Summit addresses in markdown files
echo "=== Old Summit address references ==="
rg -n "0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa" --type md
echo ""
echo "=== New Summit address references ==="
rg -n "0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc" --type mdRepository: Provable-Games/summit
Length of output: 556
🏁 Script executed:
# Check if AGENTS.md exists and show its content if it does
if [ -f "AGENTS.md" ]; then
echo "=== AGENTS.md exists, showing full content ==="
cat -n "AGENTS.md"
else
echo "AGENTS.md not found at root"
# Search for any AGENTS.md in subdirectories
find . -name "AGENTS.md" -type f 2>/dev/null
fiRepository: Provable-Games/summit
Length of output: 7871
Update AGENTS.md line 104 to the new Summit mainnet contract address.
The canonical address documentation in AGENTS.md (line 104) still references the old Summit address 0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa. Update it to match the new address 0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc from README.md. Also update client/README.md line 48 for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 83, Replace the outdated Summit mainnet contract address
in AGENTS.md (currently
0x0455c73741519a2d661cad966913ee5ccb24596c518ad67dd1d189b49c15d4fa at line ~104)
with the new address
0x01aa95ea66e7e01acf7dc3fda8be0d8661230c4c36b0169e2bab8ab4d6700dfc to match
README.md, and make the identical update in client/README.md (around line ~48)
so both documents reference the same canonical Summit contract address.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Codex Review - General Engineering ReviewNo review output was produced. |
|
Claude finished @Await-0x's task —— View job Claude Review - General Engineering Reviewrun=22314735845 attempt=1 sha=68585c079b0a9528cb5353e52310fb17be5df79f scope=general Todo List
AnalysisI reviewed the changes in scope for this PR, which focuses on files outside the core app packages ( Change Summary:
Review FindingsNo issues found. The change is a straightforward documentation update that reflects the new Summit Game contract deployment. This is a pure documentation change with no behavioral impact on CI, workflows, automation, or developer tooling. The Voyager.online link is properly updated to match the new contract address, ensuring documentation accuracy. Summary: 0 CRITICAL, 0 HIGH, 0 MEDIUM, 0 LOW, 0 INFO |
|
Claude finished @Await-0x's task —— View job Claude Review - Indexer/API Reviewrun=22314735845 attempt=1 sha=68585c079b0a9528cb5353e52310fb17be5df79f scope=indexer-api Todo List
AnalysisOnly one file within the indexer/API scope was changed:
Critical Findings[HIGH] [MEDIUM] 7-Area Review Framework Applied1. Data Correctness and Idempotency: ✅ The indexer uses proper conflict resolution with 2. Filters, Selectors, and Stream Efficiency: ✅ Contract address is correctly used as a filter in event stream configuration. Address format properly normalized. 3. Event Decoding and Serde Safety: ✅ No decoder changes. Existing decoders remain valid for new contract. 4. Finality, Reorgs, and Indexer State: ❌ Critical issue with indexer state management - namespace collision will prevent clean migration. 5. Schema, Constraints, and Migrations: ✅ No schema changes. Configuration-only change. 6. Reliability, Operations, and Config: ❌ Missing migration strategy for production state transition. 7. Testing and Verification: Migration RecommendationsThis appears to be a deployment migration requiring careful operational planning:
Summary: 0 CRITICAL, 1 HIGH, 1 MEDIUM, 0 LOW, 0 INFO |
Codex Review - General Engineering ReviewNo issues found. |
Codex Review - Indexer/API Review[HIGH] [MEDIUM] Summary: 0 CRITICAL, 1 HIGH, 1 MEDIUM, 0 LOW, 0 INFO |
1 participant
Summary
What changed
Why this change
Scope
contracts/**(Cairo/Starknet contracts)client/**(frontend client)indexer/**(indexer pipeline)api/**(backend/API)Change Type
feat(new behavior)fix(bug fix)refactor(no behavior change)perf(performance improvement)test(tests only)docs(documentation only)chore(maintenance/tooling)Validation
Commands run
Area-specific verification
If
contracts/**changedcd contracts && scarb buildcd contracts && scarb test(or targeted tests listed below)If
client/**changedcd client && pnpm lintcd client && pnpm buildIf
indexer/**orapi/**changedRisk and Rollout
Risk level
Rollout / rollback plan
Breaking Changes
Assumptions
Validation:
Exceptions
Reason:
Approval/Context:
Workarounds
Why needed:
Removal plan (owner + trigger/date):
Linked Issues
Reviewer Notes
Codex review routing reminder
contracts/**client/**indexer/**+api/**Summary by CodeRabbit