Conversation
WalkthroughThis change removes the Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Persistence
participant Storage
App->>Persistence: init(~reset)
Persistence->>Storage: isInitialized()
alt reset is true OR not initialized
Persistence->>Storage: initialize(~entities, ~generalTables, ~enums)
Persistence->>Persistence: set status Ready({cleanRun: true})
else
Persistence->>Persistence: set status Ready({cleanRun: false})
end
sequenceDiagram
participant PgStorage
participant Database
PgStorage->>Database: Check for event_sync_state table
alt Schema contains other tables but not event_sync_state
PgStorage->>PgStorage: Raise error (schema used by other app)
else
PgStorage->>Database: Drop and recreate schema
PgStorage->>Database: Create tables, enums, functions
end
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)error: failed to get Caused by: Caused by: Caused by: Caused by: ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
codegenerator/cli/npm/envio/src/PgStorage.res (1)
367-369: Enhance the error message with more specific resolution steps.The error message could be more actionable by clarifying what "drop the existing schema first" means and providing the exact command.
Js.Exn.raiseError( - `Cannot run Envio migrations on PostgreSQL schema "${pgSchema}" because it contains non-Envio tables. Running migrations would delete all data in this schema. To proceed, either drop the existing schema first with "pnpm envio local db-migrate down" or specify a different schema name using the "ENVIO_PG_PUBLIC_SCHEMA" environment variable.`, + `Cannot run Envio migrations on PostgreSQL schema "${pgSchema}" because it contains non-Envio tables. Running migrations would delete all data in this schema.\n\nTo resolve this:\n1. If you want to use this schema, first backup any important data, then drop it with: "pnpm envio local db-migrate down"\n2. Or specify a different schema name by setting the "ENVIO_PG_PUBLIC_SCHEMA" environment variable\n3. Or manually drop the schema in your database if you're certain the data is not needed`, )scenarios/test_codegen/test/lib_tests/EntityHistory_test.res (1)
6-8: Consider improving the comment clarity.The comment could be more specific about why these tables are mandatory.
-// These are mandatory tables that should be created for every schema -// it allows us to distinguish whether the schema is controlled by Envio +// These are mandatory tables that must be created for every Envio-managed schema. +// The event_sync_state table is used to distinguish Envio-controlled schemas from others.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
codegenerator/cli/npm/envio/src/Persistence.res(2 hunks)codegenerator/cli/npm/envio/src/PgStorage.res(5 hunks)codegenerator/cli/templates/static/codegen/src/db/Migrations.res(1 hunks)codegenerator/cli/templates/static/codegen/src/db/TablesStatic.res(1 hunks)scenarios/test_codegen/test/lib_tests/EntityHistory_test.res(5 hunks)scenarios/test_codegen/test/lib_tests/Persistence_test.res(1 hunks)scenarios/test_codegen/test/lib_tests/PgStorage_test.res(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.res`: Never use `[| item |]` to create an array. Use `[ item ]` instead. M...
**/*.res: Never use[| item |]to create an array. Use[ item ]instead.
Must always use=for setting value to a field. Use:=only for ref values created usingreffunction.
It's also possible to define an inline object, it'll have quoted fields in this case.
Never use %raw to access object fields if you know the type.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/rescript.mdc)
List of files the instruction was applied to:
codegenerator/cli/templates/static/codegen/src/db/Migrations.rescodegenerator/cli/templates/static/codegen/src/db/TablesStatic.rescodegenerator/cli/npm/envio/src/Persistence.resscenarios/test_codegen/test/lib_tests/EntityHistory_test.resscenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/PgStorage.resscenarios/test_codegen/test/lib_tests/PgStorage_test.res
`**/*.{res,resi}`: ReScript has record types which require a type definition bef...
**/*.{res,resi}: ReScript has record types which require a type definition beforehand. You can access record fields by dot likefoo.myField.
Use records when working with structured data, and objects to conveniently pass payload data between functions.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/rescript.mdc)
List of files the instruction was applied to:
codegenerator/cli/templates/static/codegen/src/db/Migrations.rescodegenerator/cli/templates/static/codegen/src/db/TablesStatic.rescodegenerator/cli/npm/envio/src/Persistence.resscenarios/test_codegen/test/lib_tests/EntityHistory_test.resscenarios/test_codegen/test/lib_tests/Persistence_test.rescodegenerator/cli/npm/envio/src/PgStorage.resscenarios/test_codegen/test/lib_tests/PgStorage_test.res
🧠 Learnings (6)
codegenerator/cli/templates/static/codegen/src/db/Migrations.res (2)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
codegenerator/cli/npm/envio/src/Persistence.res (2)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
scenarios/test_codegen/test/lib_tests/EntityHistory_test.res (3)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
scenarios/test_codegen/test/lib_tests/Persistence_test.res (3)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
codegenerator/cli/npm/envio/src/PgStorage.res (2)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
scenarios/test_codegen/test/lib_tests/PgStorage_test.res (3)
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
Learnt from: JonoPrest
PR: enviodev/hyperindex#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.
⏰ 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)
- GitHub Check: build_and_test
🔇 Additional comments (14)
codegenerator/cli/npm/envio/src/PgStorage.res (2)
72-74: Document the unconditional schema drop behavior.The schema is now always dropped and recreated during initialization. This is a significant behavioral change that could lead to data loss if users are not aware.
Consider adding a comment above these lines to clearly document this behavior:
let query = ref( + // IMPORTANT: This will completely drop and recreate the schema, destroying all existing data `DROP SCHEMA IF EXISTS "${pgSchema}" CASCADE; CREATE SCHEMA "${pgSchema}"; GRANT ALL ON SCHEMA "${pgSchema}" TO "${pgUser}";
214-214: Good refactoring to centralize the table name.Introducing the
eventSyncStateTableNameconstant improves maintainability and reduces the risk of typos.codegenerator/cli/templates/static/codegen/src/db/Migrations.res (1)
30-30: Correctly updated to match the new Persistence.init interface.The removal of
~skipIsInitializedCheck=truealigns with the simplified initialization logic in Persistence.res.codegenerator/cli/templates/static/codegen/src/db/TablesStatic.res (1)
24-24: Good use of the centralized table name constant.Using
PgStorage.eventSyncStateTableNameensures consistency across the codebase and reduces the risk of typos.scenarios/test_codegen/test/lib_tests/EntityHistory_test.res (1)
252-252: Test updates correctly reflect the new initialization interface.All test cases have been properly updated to:
- Include the mandatory
~generalTablesparameter- Remove the deprecated
~cleanRunparameterThe changes ensure tests remain aligned with the new storage initialization contract.
Also applies to: 601-601, 767-767, 1044-1044
codegenerator/cli/npm/envio/src/Persistence.res (1)
80-80: Clean simplification of the initialization logic.The removal of
~skipIsInitializedCheckand the simplified control flow make the initialization behavior more predictable and easier to understand. The logic now clearly states: initialize if eitherresetis true or storage is not initialized.Also applies to: 96-109
scenarios/test_codegen/test/lib_tests/Persistence_test.res (3)
36-47: LGTM: Function signature correctly updated to remove cleanRun parameter.The
initializemethod signature has been properly updated to remove thecleanRunparameter, which aligns with the PR objectives of simplifying the initialization logic.
201-203: Consistent with new initialization behavior.The test correctly expects
cleanRun: falsewhen storage is already initialized, which makes sense since no clean initialization would occur in this case.
143-145: Ignore suggestion to removecleanRunfrom tests
ThestorageStatusvariant in Persistence.res still definesReady({cleanRun: bool}), and the initialization logic sets that field internally based onshouldRun. The tests correctly assert those boolean values:
- In codegenerator/cli/npm/envio/src/Persistence.res:
type storageStatus = … | Ready({cleanRun: bool})- Branches set
persistence.storageStatus = Ready({cleanRun: true})orReady({cleanRun: false})- In scenarios/test_codegen/test/lib_tests/Persistence_test.res:
- Assertions against
Persistence.Ready({cleanRun: true})and…{cleanRun: false}match the implementationNo updates to the tests are needed.
Likely an incorrect or invalid review comment.
scenarios/test_codegen/test/lib_tests/PgStorage_test.res (5)
100-101: LGTM: Test description updated to reflect simplified initialization.The test description has been updated to reflect that the initialization now creates complete queries without conditional logic based on
cleanRun.
109-115: LGTM: Function call correctly updated to remove cleanRun parameter.The
makeInitializeTransactionfunction call has been properly updated to remove the~cleanRunparameter, which aligns with the simplified initialization logic.
127-129: LGTM: Expected SQL output reflects unconditional schema recreation.The expected SQL output now shows separate statements for schema drop/create and grants, which is consistent with the updated behavior that always drops and recreates the schema unconditionally.
179-181: LGTM: Minimal configuration test updated consistently.The minimal configuration test has been updated to match the same pattern of separate schema creation and grant statements.
215-217: LGTM: Single entity test updated consistently.The single entity test follows the same pattern of separate schema creation and grant statements, maintaining consistency across all test cases.
moose-code
left a comment
moose-code
left a comment
There was a problem hiding this comment.
Thanks - just read through the summary and this all makes good sense as next step in persitiance. LGTM!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
codegenerator/cli/src/config_parsing/chain_helpers.rs (1)
525-525: Consider removing PharosDevnet from the threshold configuration.PharosDevnet (line 525) is included in the threshold configuration but is no longer a HypersyncNetwork. This seems inconsistent - non-HypersyncNetwork variants typically wouldn't need threshold configuration.
If PharosDevnet is no longer used as a HypersyncNetwork, consider removing it from the match statement:
| Network::Curtis | Network::Worldchain | Network::Sonic - | Network::PharosDevnet | Network::Swell | Network::Taraxa => DEFAULT_CONFIRMED_BLOCK_THRESHOLD,Also applies to: 531-532
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
codegenerator/cli/src/config_parsing/chain_helpers.rs(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
codegenerator/cli/src/config_parsing/chain_helpers.rs (1)
codegenerator/cli/src/cli_args/interactive_init/evm_prompts.rs (1)
HypersyncNetwork(224-232)
⏰ 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)
- GitHub Check: build_and_test
🔇 Additional comments (3)
codegenerator/cli/src/config_parsing/chain_helpers.rs (3)
351-353: LGTM! Taraxa network addition follows conventions.The new network is correctly added with:
- Proper alphabetical ordering between Tangle and Unichain
- HypersyncNetwork subenum attribute
- Unique network ID 841
582-582: Tier assignments are consistent with network attributes.The changes correctly:
- Add Taraxa to the Bronze tier (line 582)
- Remove PharosDevnet from the Stone tier (lines 584-591)
These changes align with the HypersyncNetwork attribute modifications.
Also applies to: 584-591
262-263: Confirm Hypersync support removal is intentionalWe’ve verified that
HypersyncNetwork::MorphTestnetno longer appears anywhere in the codebase (no matches forHypersyncNetwork::MorphTestnet), so dropping the#[subenum(HypersyncNetwork)]attribute simply removes MorphTestnet from all hypersync-related commands.• If you intended to discontinue Hypersync support for MorphTestnet, no further action is needed.
• Otherwise, re-add the attribute to restore MorphTestnet in theHypersyncNetworksubenum.
2 participants
This is part of the Effects Cache project. The goal is to ensure that the schema used and managed by Envio does not contain any tables unrelated to the currently running indexer. And that we have a proper initialization step, which we can use to load persisted cache from jsons.
pnpm envio startwork without the need forpnpm envio local db-migrate up.Summary by CodeRabbit
Summary by CodeRabbit
Refactor
Tests