Platform & Storage abstractions #819
Conversation
|
This is cool |
WalkthroughReplaces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant EP as EventProcessing
participant IM as InMemoryStore
participant SZ as Persistence.storage
participant PG as PgStorage
participant DB as Postgres
EP->>IM: collect raw events / build batch
EP->>SZ: writeBatch(~batch, ~inMemoryStore=IM, ~isInReorgThreshold, ~config, ~allEntities, ~batchCache)
SZ->>PG: delegate writeBatch
alt normal
PG->>DB: run deletes / updates / inserts / set raw events / checkpoint / effect cache
DB-->>PG: success
PG-->>SZ: ok
SZ-->>EP: resolves
else error
DB-->>PG: error
PG->>SZ: map to StorageError / retry (maybe PgEncodingError -> escape & retry)
SZ-->>EP: rejects StorageError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (3)codegenerator/cli/**📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Files:
codegenerator/cli/templates/dynamic/**/*.hbs📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Files:
{codegenerator/cli/templates/static/codegen/src/**,codegenerator/cli/templates/dynamic/codegen/src/**}📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-21T16:31:22.127ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
codegenerator/cli/npm/envio/src/PgStorage.res (1)
516-540: Optional guard for empty delete batchesCallers currently filter out empties, but adding an explicit fast-path keeps this helper robust if a future caller forgets. It also avoids issuing
ANY($1::text[])with an empty payload.let deleteByIdsOrThrow = async (sql, ~pgSchema, ~ids, ~table: Table.table) => { - switch await ( + if ids->Array.length === 0 { + () + } else switch await ( switch ids { | [_] => sql->Postgres.preparedUnsafe(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
scenarios/test_codegen/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
codegenerator/cli/npm/envio/src/Config.res(3 hunks)codegenerator/cli/npm/envio/src/EventRegister.res(3 hunks)codegenerator/cli/npm/envio/src/EventRegister.resi(1 hunks)codegenerator/cli/npm/envio/src/InMemoryStore.gen.ts(1 hunks)codegenerator/cli/npm/envio/src/PgStorage.res(3 hunks)codegenerator/cli/npm/envio/src/Platform.res(1 hunks)codegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbs(1 hunks)codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs(2 hunks)codegenerator/cli/templates/dynamic/codegen/src/Types.res.hbs(0 hunks)codegenerator/cli/templates/static/codegen/src/EventProcessing.res(5 hunks)codegenerator/cli/templates/static/codegen/src/IO.res(0 hunks)codegenerator/cli/templates/static/codegen/src/UserContext.res(1 hunks)codegenerator/cli/templates/static/codegen/src/db/DbFunctionsEntities.res(0 hunks)codegenerator/cli/templates/static/codegen/src/db/DbFunctionsImplementation.js(0 hunks)codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res(4 hunks)codegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.res(1 hunks)
💤 Files with no reviewable changes (4)
- codegenerator/cli/templates/static/codegen/src/db/DbFunctionsImplementation.js
- codegenerator/cli/templates/dynamic/codegen/src/Types.res.hbs
- codegenerator/cli/templates/static/codegen/src/IO.res
- codegenerator/cli/templates/static/codegen/src/db/DbFunctionsEntities.res
🧰 Additional context used
📓 Path-based instructions (6)
codegenerator/cli/**
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
The Rust CLI lives in codegenerator/cli
Files:
codegenerator/cli/npm/envio/src/InMemoryStore.gen.tscodegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.rescodegenerator/cli/npm/envio/src/EventRegister.resicodegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbscodegenerator/cli/templates/static/codegen/src/UserContext.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.rescodegenerator/cli/npm/envio/src/PgStorage.rescodegenerator/cli/npm/envio/src/Config.rescodegenerator/cli/npm/envio/src/Platform.rescodegenerator/cli/npm/envio/src/EventRegister.res
codegenerator/cli/npm/envio/**
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Shared library-fied runtime lives in codegenerator/cli/npm/envio
Files:
codegenerator/cli/npm/envio/src/InMemoryStore.gen.tscodegenerator/cli/npm/envio/src/EventRegister.resicodegenerator/cli/npm/envio/src/PgStorage.rescodegenerator/cli/npm/envio/src/Config.rescodegenerator/cli/npm/envio/src/Platform.rescodegenerator/cli/npm/envio/src/EventRegister.res
codegenerator/cli/templates/static/**/*.res
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Static templates are raw ReScript files copied verbatim under codegenerator/cli/templates/static
Files:
codegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/static/codegen/src/UserContext.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res
{codegenerator/cli/templates/static/codegen/src/**,codegenerator/cli/templates/dynamic/codegen/src/**}
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Template versions of runtime live under templates/{static,dynamic}/codegen/src and are the recommended editing targets
Files:
codegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.rescodegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbscodegenerator/cli/templates/static/codegen/src/UserContext.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res
**/*.res
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Prefer reading ReScript .res modules directly
Files:
codegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/static/codegen/src/UserContext.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.rescodegenerator/cli/npm/envio/src/PgStorage.rescodegenerator/cli/npm/envio/src/Config.rescodegenerator/cli/npm/envio/src/Platform.rescodegenerator/cli/npm/envio/src/EventRegister.res
codegenerator/cli/templates/dynamic/**/*.hbs
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Dynamic templates are Handlebars files under codegenerator/cli/templates/dynamic
Files:
codegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs
🧠 Learnings (5)
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/EventProcessing.res : In generated runtime, EventProcessing.res handles processing
Applied to files:
codegenerator/cli/templates/static/codegen/src/EventProcessing.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/PgStorage.res : In generated runtime, PgStorage.res is part of the persistence stack
Applied to files:
codegenerator/cli/npm/envio/src/PgStorage.res
📚 Learning: 2025-05-27T17:07:04.699Z
Learnt from: JonoPrest
Repo: enviodev/hyperindex PR: 555
File: scenarios/test_codegen/test/lib_tests/EntityHistory_test.res:582-629
Timestamp: 2025-05-27T17:07:04.699Z
Learning: In ReScript test files using DbHelpers.resetPostgresClient(), this function is synchronous with signature `unit => unit` and should not be awaited. It performs a raw JavaScript reassignment of the postgres client and does not return a promise.
Applied to files:
codegenerator/cli/npm/envio/src/PgStorage.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/Config.res : In generated runtime, Config.res maps env to typed config and sets up persistence
Applied to files:
codegenerator/cli/npm/envio/src/Config.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to codegenerator/cli/npm/envio/** : Shared library-fied runtime lives in codegenerator/cli/npm/envio
Applied to files:
codegenerator/cli/npm/envio/src/Config.res
🔇 Additional comments (25)
codegenerator/cli/npm/envio/src/InMemoryStore.gen.ts (1)
1-6: LGTM! Clean type definition for raw events key.The new
rawEventsKeytype structure is well-defined with appropriate readonly modifiers and aligns with the platform abstraction changes.codegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbs (1)
100-104: LGTM! Parameter update aligns with platform abstraction.The renaming from
~ecosystemto~platformis consistent with theEventRegister.startRegistrationsignature change and correctly usesconfigWithoutRegistrations.platform.codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res (3)
233-233: LGTM! Platform-specific timestamp accessor correctly implemented.The change from
Types.Block.getTimestamptoindexer.config.platform.getTimestampproperly delegates to platform-specific behavior, improving flexibility across different blockchain platforms.
658-658: LGTM! Call sites correctly updated with indexer parameter.Both invocations of
updateProgressedChainsproperly pass~indexer=state.indexer.Also applies to: 757-757
200-200: All call sites are correctly updated with the new parameter.Verification confirms both invocations of
updateProgressedChains(lines 658 and 757 in GlobalState.res) properly pass the required~indexer=state.indexerparameter. No other call sites exist in the codebase.codegenerator/cli/templates/static/codegen/src/ink/components/CustomHooks.res (1)
37-37: LGTM! Clean migration to platform.name accessor.The change correctly sources the ecosystem value from
config.platform.namewhile maintaining the existing InitApi structure. The type coercion indicates proper variant compatibility.codegenerator/cli/templates/static/codegen/src/UserContext.res (1)
322-322: LGTM! Address validation updated to use platform abstraction.The condition correctly uses
params.config.platform.name === Evmwhile preserving the existing address validation and lowercasing behavior.codegenerator/cli/npm/envio/src/EventRegister.resi (1)
6-10: LGTM! Public API signature updated for platform abstraction.The parameter change from
~ecosystem: Config.ecosystemto~platform: Platform.tis consistent with the broader refactoring. This is a breaking change to the public API, so ensure any external consumers are aware.codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs (2)
46-50: LGTM! Test helper updated with platform parameter.The
EventRegister.startRegistrationcall correctly uses~platform=config.platforminstead of the previous~ecosystemparameter.
373-379: LGTM! Event processing updated to use platform-specific accessors.The changes correctly replace
Types.Blockgetters withconfig.platform.getTimestampandconfig.platform.getNumber, consistently applying the platform abstraction throughout event item creation and block number tracking.codegenerator/cli/npm/envio/src/Config.res (4)
48-48: LGTM! Config type updated to platform abstraction.The replacement of
ecosystemwithplatform: Platform.tin the public typetis the foundation of this refactoring and enables platform-specific behavior throughout the system.
63-63: LGTM! Parameter type and default value are appropriate.The parameter
~ecosystem: Platform.name=Platform.Evmproperly defaults to EVM and accepts a platform name that will be converted to a Platform.t instance.
95-108: LGTM! Config construction properly uses platform.The return object correctly includes the
platformfield (line 102) derived from thefromNametranslation, completing the migration from ecosystem to platform abstraction.
93-93: Code is correct—all ecosystem values are properly handled.The
Platform.fromNamefunction correctly handles all ecosystem values. The type system guarantees completeness:
- The
Platform.nametype has exactly two variants: Evm and Fuel- The
fromNamefunction's switch statement covers both variants- The
ecosystemparameter is typed asPlatform.name, ensuring only valid values reach the function- ReScript's exhaustive pattern matching enforces completeness at compile time
No additional handling needed.
codegenerator/cli/templates/static/codegen/src/EventProcessing.res (3)
50-50: LGTM! Platform abstraction properly integrated.The addition of the
~configparameter enables platform-specific block field cleanup and ID retrieval, making the function work correctly across EVM and Fuel platforms.Also applies to: 64-64, 88-88
223-223: LGTM! Call site correctly updated.The function call properly passes the config parameter from the indexer.
424-458: LGTM! Error handling properly centralized.The refactored error handling around the DB write provides better exception management with specific handling for
Persistence.StorageErrorand a fallback for unexpected errors.codegenerator/cli/npm/envio/src/EventRegister.res (1)
7-7: LGTM! Platform field migration is consistent.The migration from
ecosystemtoplatform: Platform.tis properly applied across the type definition, function signature, and record construction.Also applies to: 39-41
codegenerator/cli/npm/envio/src/Platform.res (4)
1-14: LGTM! Well-designed platform abstraction types.The
Platform.ttype provides a clean interface for platform-specific behavior, with thenamevariant enabling type-safe platform identification and accessor functions supporting polymorphic operations.
16-26: LGTM! Platform-specific modules properly implemented.The Evm and Fuel modules correctly define external bindings for their respective block field names and provide efficient in-place cleanup functions using raw JavaScript.
Also applies to: 103-113
28-101: LGTM! Platform constants comprehensively defined.The
evmandfuelconstants provide complete field lists, including EVM's extensive block/transaction fields (with L2-specific additions) and Fuel's simpler structure, properly wired to their respective module implementations.Also applies to: 115-126
128-133: LGTM! Clean factory function.The
fromNamefunction provides a straightforward way to obtain the appropriate platform instance from anamevariant.codegenerator/cli/npm/envio/src/PgStorage.res (3)
179-185: Delete helpers match existing read helpersThe single- vs multi-id shape mirrors
makeLoadById(s)above, so the storage API stays symmetrical and easy to reason about. Nice touch.
953-963: Nice abstraction for conditional batch writesWrapping the "only touch SQL when there’s work" pattern keeps Promise plumbing tidy in the transactional path. Reads well.
965-1301: Retry + escape loop looks solidThe targeted
PgEncodingErrorretry with table-scoped UTF-8 cleanup, plus the sharedspecificErrorhandoff, gives us a predictable path to recovery without double-reporting errors. Thanks for threading the history backfill, deletes, and effect cache in a single transactional view.
| switch registration.ecosystem { | ||
| | Evm => () | ||
| | Fuel => | ||
| if registration.platform !== Platform.evm { |
There was a problem hiding this comment.
Fix object reference comparison for platform check.
The comparison registration.platform !== Platform.evm uses reference inequality (!==), which compares object identities rather than values. This is fragile and may fail if Platform.t instances are recreated anywhere in the codebase.
Apply this diff to compare the platform name instead:
- if registration.platform !== Platform.evm {
+ if registration.platform.name !== Platform.Evm {This compares the name enum variant, which is the correct semantic check and is robust regardless of how Platform.t instances are constructed.
📝 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.
| if registration.platform !== Platform.evm { | |
| if registration.platform.name !== Platform.Evm { |
🤖 Prompt for AI Agents
In codegenerator/cli/npm/envio/src/EventRegister.res around line 84, the check
uses reference inequality "registration.platform !== Platform.evm" which
compares object identity; replace it with a value comparison of the enum variant
name, e.g. use "registration.platform.name !== Platform.evm.name" (or
pattern-match on Platform.evm) so the platform comparison is based on the
variant value rather than object identity.
There was a problem hiding this comment.
@DZakh I assume this is intended behaviour? They are global objects?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Eah, it'll be fine in this case, but ideally to do it as rabbit suggests. Although, the code is removed in the next PR, so we are fine.
| // Execute raw SQL query | ||
| executeUnsafe: string => promise<unknown>, | ||
| // Check if entity history has rows | ||
| hasEntityHistoryRows: unit => promise<bool>, | ||
| // Update chain metadata | ||
| setChainMeta: dict<InternalTable.Chains.metaFields> => promise<unknown>, | ||
| // Prune old checkpoints | ||
| pruneStaleCheckpoints: int => promise<unit>, | ||
| // Prune stale entity history | ||
| pruneStaleEntityHistory: ( | ||
| ~entityName: string, | ||
| ~entityIndex: int, | ||
| ~safeCheckpointId: int, | ||
| ) => promise<unit>, | ||
| // Get rollback target checkpoint | ||
| getRollbackTargetCheckpoint: ( | ||
| ~reorgChainId: int, | ||
| ~lastKnownValidBlockNumber: int, | ||
| ) => promise<array<{"id": int}>>, | ||
| // Get rollback progress diff | ||
| getRollbackProgressDiff: int => promise< | ||
| array<{ | ||
| "chain_id": int, | ||
| "events_processed_diff": string, | ||
| "new_progress_block_number": int, | ||
| }>, | ||
| >, | ||
| // Get rollback data for entity | ||
| getRollbackData: ( | ||
| ~entityConfig: Internal.entityConfig, | ||
| ~rollbackTargetCheckpointId: int, | ||
| ) => promise<(array<{"id": string}>, array<unknown>)>, | ||
| // Write batch to storage | ||
| writeBatch: ( | ||
| ~batch: Batch.t, | ||
| ~inMemoryStore: InMemoryStore.t, | ||
| ~isInReorgThreshold: bool, | ||
| ~config: Config.t, | ||
| ~allEntities: array<Internal.entityConfig>, | ||
| ~batchCache: array<effectBatchCache>, | ||
| ) => promise<unit>, |
There was a problem hiding this comment.
This is not very reusable, but we'll be able to clean it up with time
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
codegenerator/cli/templates/static/codegen/src/EventProcessing.res (1)
437-468: Consider extracting batchCache construction into a helper function.The batchCache construction logic is correct but deeply nested, which impacts readability. Consider extracting it into a separate function like
buildBatchCacheFromEffects.For example:
let buildBatchCacheFromEffects = ( effects: Js.Dict.t<'a>, persistence: Persistence.t, ) => { effects ->Js.Dict.keys ->Belt.Array.keepMapU(effectName => { let inMemTable = effects->Js.Dict.unsafeGet(effectName) let {idsToStore, dict, effect, invalidationsCount} = inMemTable switch idsToStore { | [] => None | ids => { let items = Belt.Array.makeUninitializedUnsafe(ids->Belt.Array.length) ids->Belt.Array.forEachWithIndex((index, id) => { items->Js.Array2.unsafe_set( index, ({id, output: dict->Js.Dict.unsafeGet(id)}: Internal.effectCacheItem), ) }) Some( persistence->Persistence.getEffectBatchCache( ~effect, ~items, ~invalidationsCount, ), ) } } }) }Then use it in the writeBatch call:
~batchCache=buildBatchCacheFromEffects( inMemoryStore.effects, indexer.persistence, )scenarios/test_codegen/test/helpers/Mock.res (1)
187-205: Verify test coverage for new storage methods.The new storage method stubs return empty/default values. While appropriate for basic mocking, ensure that tests requiring realistic behavior from these methods (especially
writeBatch,getRollbackData, andgetRollbackProgressDiff) have dedicated test implementations or use actual storage.Consider whether any existing or planned tests need more sophisticated mock implementations for these methods to properly validate rollback and batch-write logic.
codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs (1)
590-605: Mock storage implementations are minimal.Similar to the main Mock.res file, these storage method stubs return empty/default values. Ensure tests that exercise batch writing and rollback logic have appropriate mock behavior or use real storage.
codegenerator/cli/npm/envio/src/Persistence.res (1)
86-127: Storage interface significantly expanded.The addition of
executeUnsafe,hasEntityHistoryRows,setChainMeta, pruning methods, and rollback helpers centralizes storage operations. However, some return types useunknownwhich reduces type safety.Consider strengthening return types where possible:
- Line 87:
executeUnsafecould returnpromise<array<unknown>>to indicate it returns rows- Line 91:
setChainMetacould returnpromise<unit>instead ofpromise<unknown>These changes would improve type safety and documentation.
Also applies to: 142-143
codegenerator/cli/npm/envio/src/PgStorage.res (2)
554-868: Complex batch write implementation with retry logic.The
writeBatchfunction is comprehensive but very large (~315 lines). Key observations:Strengths:
- Proper transaction boundaries with
Postgres.beginSql- Handles history backfilling before updates (lines 645-654)
- UTF-8 encoding error retry with targeted escaping (lines 848-867)
- Normalizes exception types for consistent handling (lines 718-722)
- Separates effect cache writes from main transaction (lines 824-830)
Complexity concerns:
- The
specificErrorref pattern (line 569) for error propagation through nested async operations is intricate- Nested try-catch blocks with different error handling strategies
- The UTF-8 retry mechanism adds another layer of control flow
Consider extracting helper functions for:
- History processing (lines 612-688):
processEntityHistory- Entity sets and deletes (lines 691-708):
processEntityUpdates- Error normalization logic (lines 718-745):
normalizeStorageErrorThis would improve readability and testability without changing functionality. However, given this is a major refactor introducing the new storage API, these improvements can be deferred to a follow-up PR.
1271-1300: Potential edge case in hasEntityHistoryRows.The implementation checks if any history table has rows, but:
- Line 1279: Returns
falseif no history tables exist- Lines 1286-1295: Swallows exceptions and returns
falsefor each table checkThis might hide legitimate errors (e.g., permission issues). Consider logging exceptions before returning
false, or allowing certain exceptions to propagate.Example improvement:
} catch { - | _ => false + | exn => { + Logging.warn({ + "msg": "Failed to check entity history rows", + "table": table["table_name"], + "error": exn + }) + false + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
codegenerator/cli/npm/envio/src/Persistence.res(5 hunks)codegenerator/cli/npm/envio/src/PgStorage.res(4 hunks)codegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbs(1 hunks)codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs(3 hunks)codegenerator/cli/templates/static/codegen/src/EventProcessing.res(5 hunks)codegenerator/cli/templates/static/codegen/src/IO.res(1 hunks)codegenerator/cli/templates/static/codegen/src/db/Db.res(0 hunks)codegenerator/cli/templates/static/codegen/src/db/Migrations.res(1 hunks)codegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.res(1 hunks)codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res(8 hunks)scenarios/test_codegen/test/helpers/DbHelpers.res(1 hunks)scenarios/test_codegen/test/helpers/Mock.res(1 hunks)scenarios/test_codegen/test/lib_tests/Persistence_test.res(2 hunks)scenarios/test_codegen/test/schema_types/BigDecimal_test.res(1 hunks)scenarios/test_codegen/test/schema_types/Timestamp_test.res(1 hunks)
💤 Files with no reviewable changes (1)
- codegenerator/cli/templates/static/codegen/src/db/Db.res
🧰 Additional context used
📓 Path-based instructions (6)
**/*.res
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Prefer reading ReScript .res modules directly
Files:
scenarios/test_codegen/test/schema_types/Timestamp_test.rescodegenerator/cli/templates/static/codegen/src/db/Migrations.resscenarios/test_codegen/test/helpers/DbHelpers.rescodegenerator/cli/templates/static/codegen/src/IO.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.resscenarios/test_codegen/test/helpers/Mock.resscenarios/test_codegen/test/schema_types/BigDecimal_test.rescodegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.resscenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/Persistence.rescodegenerator/cli/npm/envio/src/PgStorage.res
codegenerator/cli/**
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
The Rust CLI lives in codegenerator/cli
Files:
codegenerator/cli/templates/static/codegen/src/db/Migrations.rescodegenerator/cli/templates/static/codegen/src/IO.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.rescodegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.rescodegenerator/cli/npm/envio/src/Persistence.rescodegenerator/cli/npm/envio/src/PgStorage.rescodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs
codegenerator/cli/templates/static/**/*.res
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Static templates are raw ReScript files copied verbatim under codegenerator/cli/templates/static
Files:
codegenerator/cli/templates/static/codegen/src/db/Migrations.rescodegenerator/cli/templates/static/codegen/src/IO.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.rescodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res
{codegenerator/cli/templates/static/codegen/src/**,codegenerator/cli/templates/dynamic/codegen/src/**}
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Template versions of runtime live under templates/{static,dynamic}/codegen/src and are the recommended editing targets
Files:
codegenerator/cli/templates/static/codegen/src/db/Migrations.rescodegenerator/cli/templates/static/codegen/src/IO.rescodegenerator/cli/templates/static/codegen/src/EventProcessing.rescodegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.rescodegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/static/codegen/src/globalState/GlobalState.rescodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs
codegenerator/cli/templates/dynamic/**/*.hbs
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Dynamic templates are Handlebars files under codegenerator/cli/templates/dynamic
Files:
codegenerator/cli/templates/dynamic/codegen/src/Generated.res.hbscodegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs
codegenerator/cli/npm/envio/**
📄 CodeRabbit inference engine (.cursor/rules/navigation.mdc)
Shared library-fied runtime lives in codegenerator/cli/npm/envio
Files:
codegenerator/cli/npm/envio/src/Persistence.rescodegenerator/cli/npm/envio/src/PgStorage.res
🧠 Learnings (8)
📚 Learning: 2025-05-27T17:07:04.699Z
Learnt from: JonoPrest
Repo: enviodev/hyperindex PR: 555
File: scenarios/test_codegen/test/lib_tests/EntityHistory_test.res:582-629
Timestamp: 2025-05-27T17:07:04.699Z
Learning: In ReScript test files using DbHelpers.resetPostgresClient(), this function is synchronous with signature `unit => unit` and should not be awaited. It performs a raw JavaScript reassignment of the postgres client and does not return a promise.
Applied to files:
scenarios/test_codegen/test/schema_types/Timestamp_test.rescodegenerator/cli/templates/static/codegen/src/db/Migrations.resscenarios/test_codegen/test/helpers/DbHelpers.rescodegenerator/cli/npm/envio/src/PgStorage.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/PgStorage.res : In generated runtime, PgStorage.res is part of the persistence stack
Applied to files:
codegenerator/cli/templates/static/codegen/src/db/Migrations.resscenarios/test_codegen/test/helpers/DbHelpers.resscenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/Persistence.rescodegenerator/cli/npm/envio/src/PgStorage.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/IO.res : In generated runtime, IO.res is used by persistence and processing
Applied to files:
codegenerator/cli/templates/static/codegen/src/IO.res
📚 Learning: 2025-05-27T17:07:12.878Z
Learnt from: JonoPrest
Repo: enviodev/hyperindex PR: 555
File: scenarios/test_codegen/test/lib_tests/EntityHistory_test.res:631-659
Timestamp: 2025-05-27T17:07:12.878Z
Learning: In isolated tests for EntityHistory rollbacks with controlled mock data setup, the ordering of getRollbackDiff results is deterministic and doesn't require sorting before comparison with deepStrictEqual.
Applied to files:
codegenerator/cli/templates/static/codegen/src/IO.resscenarios/test_codegen/test/helpers/Mock.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/EventProcessing.res : In generated runtime, EventProcessing.res handles processing
Applied to files:
codegenerator/cli/templates/static/codegen/src/EventProcessing.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/ChainManager.res : In generated runtime, ChainManager.res is part of the fetch side
Applied to files:
codegenerator/cli/templates/static/codegen/src/eventFetching/ChainManager.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/Persistence.res : In generated runtime, Persistence.res is part of the persistence stack
Applied to files:
scenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/Persistence.res
📚 Learning: 2025-10-21T16:31:22.127Z
Learnt from: CR
Repo: enviodev/hyperindex PR: 0
File: .cursor/rules/navigation.mdc:0-0
Timestamp: 2025-10-21T16:31:22.127Z
Learning: Applies to generated/src/Hasura.res : In generated runtime, Hasura.res is part of the persistence stack
Applied to files:
scenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/Persistence.res
🔇 Additional comments (14)
codegenerator/cli/templates/static/codegen/src/EventProcessing.res (3)
50-54: LGTM! Platform abstraction properly integrated.The addition of the
~configparameter and usage ofconfig.platform.cleanUpRawEventFieldsInPlace(line 68) andconfig.platform.getId(line 92) correctly implements the platform abstraction pattern described in the PR objectives.Also applies to: 68-68, 92-92
227-229: LGTM! Call site updated correctly.The call to
addItemToRawEventsnow properly passes the~config=indexer.configparameter, matching the updated function signature.
430-495: LGTM! Unified batch writing with proper error handling.The refactor successfully centralizes batch writing logic with the new
storage.writeBatchcall. The error handling appropriately distinguishes betweenPersistence.StorageErrorand generic exceptions, and timing metrics are correctly preserved.scenarios/test_codegen/test/lib_tests/Persistence_test.res (1)
7-7: LGTM! API usage correctly updated.The removal of the
sqlparameter aligns with the new persistence API surface where storage methods are accessed throughpersistence.storageinstead of direct SQL access.Also applies to: 133-133
codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs (2)
47-47: Platform abstraction correctly applied.The parameter change from
~ecosystemto~platformaligns with the PR's platform abstraction goal.
373-379: No compatibility issues found—the code change is correct.The Platform.t interface properly defines both
getTimestampandgetNumbermethods with signatures acceptingInternal.eventBlockobjects:type t = { getNumber: Internal.eventBlock => int, getTimestamp: Internal.eventBlock => int, // ... other fields }Both methods are implemented in platform-specific modules (Evm and Fuel) and return integers as expected. The usage
event.block->config.platform.getTimestampandevent.block->config.platform.getNumberis fully compatible with these signatures.codegenerator/cli/templates/static/codegen/src/globalState/GlobalState.res (3)
198-198: Function signature change propagated correctly.The addition of
~indexer: Indexer.tparameter toupdateProgressedChainsand its usage at all call sites (lines 656, 755) maintains consistency. This enables platform-based timestamp access within the function.Also applies to: 656-656, 755-755
231-231: Platform-based timestamp access.Accessing block timestamp via
indexer.config.platform.getTimestamp(eventItem.event.block)correctly uses the new platform abstraction. This provides better support for different blockchain platforms (EVM, Fuel, etc.).
190-191: Storage API migration completed successfully.The migration from direct SQL calls to storage-based methods is comprehensive:
setChainMetareplaces SQL-based chain metadata updatespruneStaleCheckpointsandpruneStaleEntityHistoryreplace SQL-based pruninggetRollbackTargetCheckpointandgetRollbackProgressDiffreplace SQL-based rollback queriesThis abstraction improves testability and separates storage concerns from business logic.
Also applies to: 821-821, 833-837, 1020-1037
codegenerator/cli/npm/envio/src/Persistence.res (2)
39-44: Good separation of concerns.The new
effectBatchCachetype enables callers to determine cache initialization strategy while keeping the cache metadata management within Persistence. This is a cleaner API than havinggetEffectBatchCache(formerlysetEffectCacheOrThrow) directly call storage.
242-264: API refactoring verified as correct.The return value from
getEffectBatchCacheis properly handled throughout the call chain:
- EventProcessing.res (line 459) calls
getEffectBatchCacheand passes the result towriteBatchvia the~batchCacheparameter- PgStorage.res (lines 826-830) receives
batchCacheand correctly delegates persistence by callingsetEffectCacheOrThrowfor each cache itemThe refactoring successfully shifts responsibility to the caller as intended, with no unhandled return values.
codegenerator/cli/npm/envio/src/PgStorage.res (3)
179-185: Delete operations properly implemented.The delete query builders and
deleteByIdsOrThrowcorrectly handle both single and batch deletions, with appropriate error wrapping inPersistence.StorageError.Also applies to: 516-540
542-552: Good optimization pattern.The
executeSethelper avoids unnecessary database calls when the items array is empty. This is a useful micro-optimization for batch operations.
1269-1390: Storage method implementations look correct.All new storage methods are properly implemented:
executeUnsafe: Direct SQL passthrough (line 1269)hasEntityHistoryRows: Checks for entity history data (lines 1271-1300)setChainMeta: Wraps chain metadata update (lines 1302-1305)- Rollback helpers: Properly delegate to internal modules (lines 1307-1350)
writeBatchMethod: Wrapper for internal writeBatch (lines 1352-1371)
| let deleteByIdsOrThrow = async (sql, ~pgSchema, ~ids, ~table: Table.table) => { | ||
| switch await ( | ||
| switch ids { | ||
| | [_] => |
There was a problem hiding this comment.
Should we handle the empty case here?
There was a problem hiding this comment.
Is there any logic in this file that's changed? Or all just moved from IO.res?
There was a problem hiding this comment.
All moved from IO + moved pgSchema and all direct access to Db to the storage methods
| blockFields: [ | ||
| "number", | ||
| "timestamp", | ||
| "hash", |
There was a problem hiding this comment.
Haven't found where this is used yet but would love it if this would come from the client.
There was a problem hiding this comment.
Not used yet, but I plan to use it to create a prototype for the block and transaciton object. What do you mean by client?
| "sendRoot", | ||
| "mixHash", | ||
| ], | ||
| transactionFields: [ |
There was a problem hiding this comment.
Also just noting, some of these fields are only hypersync transactions. Or otherwise it's a combination of transactions and transactionReceipts on regular RPC
| hasEntityHistoryRows: () => Promise.resolve(false), | ||
| setChainMeta: _ => Promise.resolve(%raw(`undefined`)), | ||
| pruneStaleCheckpoints: _ => Promise.resolve(), | ||
| pruneStaleEntityHistory: (~entityName as _, ~entityIndex as _, ~safeCheckpointId as _) => Promise.resolve(), | ||
| getRollbackTargetCheckpoint: (~reorgChainId as _, ~lastKnownValidBlockNumber as _) => Promise.resolve([]), | ||
| getRollbackProgressDiff: _ => Promise.resolve([]), | ||
| getRollbackData: (~entityConfig as _, ~rollbackTargetCheckpointId as _) => Promise.resolve(([], [])), | ||
| writeBatch: ( | ||
| ~batch as _, | ||
| ~inMemoryStore as _, | ||
| ~isInReorgThreshold as _, | ||
| ~config as _, | ||
| ~allEntities as _, | ||
| ~batchCache as _, | ||
| ) => Promise.resolve(), |
There was a problem hiding this comment.
Should these maybe reject? Or do they get called during a test?
There was a problem hiding this comment.
Makes sense to make them fail
| inMemoryStore.effects | ||
| ->Js.Dict.keys | ||
| ->Belt.Array.keepMapU(effectName => { | ||
| let inMemTable = inMemoryStore.effects->Js.Dict.unsafeGet(effectName) | ||
| let {idsToStore, dict, effect, invalidationsCount} = inMemTable | ||
| switch idsToStore { | ||
| | [] => None | ||
| | ids => { |
There was a problem hiding this comment.
Is this always a function on inMemoryStore? Perhaps this should be done inside of writeBatchFunction since inMemoryStore is passed in
There was a problem hiding this comment.
Turn this file into a Rollback module or remove
This PR is a preparation for:
Summary by CodeRabbit
New Features
Refactor
Tests
Chores