Fix Highlight Base Ingestor

[pbkompasz]

User description

Fix ingestor (#62).

Notes

All tests pass for Highlight ingestor.
Fixed the API requests to match the new responses.
In case of a successful mint transaction some internal transactions always fail. I added a check for this in the dry runner:

const totalCalls = simulationResult.rawSimulationResult.calls.length;
const failedCalls = simulationResult.rawSimulationResult.calls.filter(
  (call: { error: any }) => call.error,
).length;
if (totalCalls - failedCalls > 0) {
  console.log(`✅ Although one or more (${failedCalls}) Error Occurred [execution reverted] Contract Execution Completed`);
} else {
  console.log('❌ Simulation Failed');
}

Example successful transactions with failed internal txns: #1, #2

---

PR Type

Bug fix, Tests

---

Description

• Fixed the Highlight ingestor to support the updated API and URL formats.
• Enhanced simulation handling by checking for failed internal transactions and improving logging.
• Updated tests for the Highlight ingestor to ensure proper functionality.
• Added new type definitions to support additional collection data.

---

Changes walkthrough 📝

	Relevant files
Enhancement	index.ts Enhance simulation result handling and logging                      src/dry-run/index.ts Added logic to handle failed internal transactions in simulation results. Improved logging for simulation results. +10/-2    types.ts Extend collection types with additional data                          src/ingestors/highlight/types.ts Added new type CollectionByAddress3 for additional collection data. +14/-1
Bug fix	index.ts Fix and enable Highlight ingestor URL support                        src/ingestors/highlight/index.ts Re-enabled and fixed URL support logic for Highlight ingestor. Corrected collection ID and contract address handling. +25/-27  offchain-metadata.ts Update collection ID format in API requests                            src/ingestors/highlight/offchain-metadata.ts Updated collection ID format for API requests. +2/-2
Tests	highlight.test.ts Enable and update tests for Highlight ingestor                      test/ingestors/highlight.test.ts Enabled previously skipped tests for Highlight ingestor. Adjusted test expectations for contract parameters. +5/-5

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit a580156)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 62 - Partially compliant Compliant requirements: Fixed Highlight ingestor for Base mints Supports both open edition and generative mints (based on test cases) Updated API integration with new URL formats and responses Non-compliant requirements: ETH network support is not implemented (code only handles Base chainId 8453)

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The new error handling logic for failed internal transactions needs validation to ensure it correctly identifies successful mints despite internal failures Data Validation The collection ID fallback logic might need review to ensure proper handling of all possible collection data structures API Integration Verify that the new collection ID format with 'base:' prefix works for all API endpoints

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to a580156
Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add validation for required contract address to prevent runtime errors Add null check for collection.primaryContract before using it in setMintOutputContract to prevent potential runtime errors. src/ingestors/highlight/index.ts [96] +if (!collection.primaryContract) { + throw new MintIngestorError(MintIngestionErrorName.MissingRequiredData, 'Primary contract address not available'); +} mintBuilder.setMintOutputContract({ chainId: 8453, address: collection.primaryContract }); Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding null check for primaryContract is crucial to prevent runtime errors and provides clear error messaging. This is an important defensive programming practice for a critical field.	8
	Handle invalid URL formats gracefully to prevent application crashes Add URL validation try-catch block in supportsUrl to handle invalid URL formats that could cause runtime errors. src/ingestors/highlight/index.ts [29-32] -const urlPattern = /^https:\/\/highlight\.xyz\/mint\/[a-f0-9]{24}$/; -return ( - new URL(url).hostname === 'www.highlight.xyz' || new URL(url).hostname === 'highlight.xyz' || urlPattern.test(url) -); +try { + const urlPattern = /^https:\/\/highlight\.xyz\/mint\/[a-f0-9]{24}$/; + const parsedUrl = new URL(url); + return parsedUrl.hostname === 'www.highlight.xyz' || parsedUrl.hostname === 'highlight.xyz' || urlPattern.test(url); +} catch (error) { + return false; +} Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding try-catch for URL parsing is important for application stability, as invalid URLs could cause crashes. The suggestion properly handles the error case by returning false.	7
General	Ensure consistent logging of simulation results for better debugging capabilities Move the console.log of simulation result outside the else block to ensure it's always logged, regardless of success or failure status. This helps with debugging both successful and failed cases. src/dry-run/index.ts [59-72] if (simulationResult.success) { console.log('✅ Simulation Success'); } else { const totalCalls = simulationResult.rawSimulationResult.calls.length; const failedCalls = simulationResult.rawSimulationResult.calls.filter( (call: { error: any }) => call.error, ).length; if (totalCalls - failedCalls > 0) { console.log(`✅ Although one or more (${failedCalls}) Error Occurred [execution reverted] Contract Execution Completed`); } else { console.log('❌ Simulation Failed'); } - console.log(JSON.stringify(simulationResult, null, 2)); } +console.log(JSON.stringify(simulationResult, null, 2)); Apply this suggestion Suggestion importance[1-10]: 5 Why: Moving the simulation result logging outside the else block would improve debugging by consistently showing the full result for both success and failure cases. The change is straightforward and enhances code maintainability.	5

💡 Need additional feedback ? start a PR chat

---

Previous suggestions

✅ Suggestions up to commit a580156

Category	Suggestion	Score
General	Improve error message clarity to better reflect partial failure states The success message for partially failed calls is misleading and could hide critical errors. Revise the message to clearly indicate that the simulation had partial failures that might affect functionality. src/dry-run/index.ts [66-67] if (totalCalls - failedCalls > 0) { - console.log(`✅ Although one or more (${failedCalls}) Error Occurred [execution reverted] Contract Execution Completed`); + console.log(`⚠️ Warning: ${failedCalls}/${totalCalls} calls failed. Partial success but functionality may be impaired`); } Suggestion importance[1-10]: 7 Why: The suggestion improves error message clarity by replacing the ambiguous "✅" success indicator with a warning symbol and providing more precise information about the failure ratio, which helps better understand the simulation results.	7

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit a580156

[chrismaddern]

/review

[qodo-merge-pro]

Persistent review updated to latest commit a580156

[chrismaddern]

@pbkompasz this is accepted. 🥳

Can I ask you to reply on our thread to confirm payment?