Skip to content

Support custom configs in MockIndexer and add envio package tests - #1445

Merged
DZakh merged 10 commits into
mainfrom
claude/configyaml-mockindexer-unify-xmdo4l
Jul 21, 2026
Merged

Support custom configs in MockIndexer and add envio package tests#1445
DZakh merged 10 commits into
mainfrom
claude/configyaml-mockindexer-unify-xmdo4l

Conversation

@DZakh

@DZakh DZakh commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

Enable MockIndexer to accept custom configs parsed from user YAML instead of always using the generated project config. Add test infrastructure to the envio package and introduce a test demonstrating YAML-driven indexer configuration.

Key Changes

  • MockIndexer lazy initialization: Convert defaultPersistence from eager to lazy evaluation via a ref, preventing database connections for tests that don't use it
  • Custom config support: Add optional ~config parameter to MockIndexer.Indexer.make and InMemoryStore.make, with fallback to generated config
  • Handler registration: When a custom config is supplied, use HandlerRegister inline registration instead of HandlerLoader.registerAllHandlers (which requires handler files on disk)
  • Contract-aware mock registrations: Pass ~contractName to makeMockSourceRegistration, using the first contract from chain config or defaulting to "MockContract"
  • Extracted helper: Add entityConfigByName function to reduce duplication in entity config lookups
  • Test infrastructure:
    • Add vitest.config.ts to envio package with sequential test execution and proper module externalization
    • Add test/setup.ts to initialize logging for all tests
    • Add YamlConfigIndexer_test.res demonstrating YAML config parsing and indexer execution
    • Update rescript.json to include test directory
    • Add pnpm test script to package.json
  • Build artifact cleanup: Strip test sources from published rescript.json via new stripTestSources function
  • CI integration: Add envio package test step to build_and_verify workflow

Implementation Details

The custom config flow allows tests to:

  1. Parse a user-defined schema and config YAML via MockIndexerConfig.parseYaml
  2. Pass the parsed config to MockIndexer.Indexer.make
  3. Register inline handlers through the public registry lifecycle instead of loading from disk

Mock source registrations now use actual contract names from the config, keeping synthetic registrations in the correct address-dependent fetch partition alongside real contracts.

https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh

Summary by CodeRabbit

  • New Features

    • Added YAML-based configuration support for mock indexer testing, including inline schemas and custom data sources.
    • Added a dedicated test command for the Envio package.
    • Published artifacts now include testing utilities without test-source references.
  • Tests

    • Expanded coverage for configuration parsing, address filters, storage, synchronization, reorganization handling, metrics, throttling, and blockchain-specific behavior.
    • Added end-to-end validation for YAML-driven indexing and entity queries.
    • Improved automated test execution in CI.

- Add packages/envio/test as a ReScript source dir with its own vitest
  setup; npm artifact excludes it (build-artifact strips the test source
  entry from the published rescript.json and never copies the dir).
- Move MockIndexerConfig and scenario-unrelated tests (ConfigYaml,
  ColumnNameFormat, Utils, ReorgDetection, HyperSync, etc.) from
  scenarios/test_codegen to packages/envio/test.
- MockIndexer.Indexer.make accepts ~config parsed through the real user
  YAML pipeline; handler-file autoload is skipped for supplied configs
  and the mock source registration derives its contract from the config
  instead of hardcoding Gravatar. The module-level pg client is now lazy.
- Add YamlConfigIndexer_test: YAML + schema strings drive a full mock
  indexer run into Postgres.
- CI runs the envio package tests (NODE_PATH points require() at the
  prebuilt NAPI addon artifact).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds Envio package test execution, Vitest configuration, publish-time test-source cleanup, broad runtime and configuration coverage, HyperSync/SVM tests, and YAML-driven MockIndexer support for scenario tests.

Changes

Envio test and publish wiring

Layer / File(s) Summary
Package test and publish wiring
.github/workflows/*, package.json, packages/envio/package.json, packages/envio/vitest.config.ts, packages/envio/test/setup.ts, packages/envio/rescript.json, packages/build-envio/src/build-artifact.ts
Adds Envio test commands and CI execution, configures Vitest and logger setup, publishes testing helpers, and removes the test source entry from published rescript.json.

Configuration and schema contracts

Layer / File(s) Summary
Configuration and schema validation
packages/envio/test/ConfigYaml_test.res, packages/envio/test/Config_test.res, packages/envio/test/OnBlockSchema_test.res
Covers YAML parsing, interpolation, ecosystem validation, schema directives, virtual ABI/IDL handling, field exhaustiveness, ABI parameter schemas, and block-filter parsing.
Address and field-selection contracts
packages/envio/test/ClientAddressFilter_test.res, packages/envio/test/AddressLowercase_test.res, packages/envio/test/BlockStore_test.res
Tests address normalization, client address-filter parsing and generated predicates, effective block gating, and Rust/ReScript block-field ordering.

Runtime behavior and storage coverage

Layer / File(s) Summary
Runtime, storage, and utility behavior
packages/envio/test/EntityFilter_test.res, packages/envio/test/Encoders_schema_test.res, packages/envio/test/lib_tests/*
Adds coverage for entity filters, nullable encoding, materialization, column naming, configuration diffs, effect caches, metrics, throttling, utilities, rate limits, and reorg detection.

HyperSync and ecosystem source coverage

Layer / File(s) Summary
HyperSync, SVM, and transaction contracts
packages/envio/test/HyperSync*_test.res, packages/envio/test/SvmHyperSync*_test.res, packages/envio/test/EventRouter_svm_test.res, packages/envio/test/TransactionStore_test.res
Tests live and mocked HyperSync queries, authorization and missing-field handling, SVM instruction filtering, event routing, and transaction field-mask contracts.

YAML-driven MockIndexer scenarios

Layer / File(s) Summary
Configurable MockIndexer and scenario integration
packages/envio/testing/MockIndexerConfig.res, scenarios/test_codegen/test/helpers/MockIndexer.res, scenarios/test_codegen/test/YamlConfigIndexer_test.res, scenarios/test_codegen/test/*
Adds YAML parsing into typed configuration, lazy configurable persistence, custom handler registration, contract-aware mock events, restart propagation, and an end-to-end YAML indexing test.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: custom MockIndexer config support and new envio package tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (6)
packages/envio/test/ConfigYaml_test.res (1)

33-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Collapse each test to one whole-value assertion.

Build a tuple or record containing the relevant config, chain, source, contract, and event values, then compare it with one toEqual call.

As per coding guidelines, “Always use single assert to check the whole value instead of multiple asserts for every field.”

Also applies to: 66-98, 807-831, 881-895

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/ConfigYaml_test.res` around lines 33 - 64, Update the
affected tests, including the case around MockIndexerConfig.parseYaml and the
ranges noted in the review, to gather all relevant config, chain, source,
contract, and event fields into one tuple or record and verify them with a
single toEqual assertion. Remove the individual field-level expect calls while
preserving the existing expected values and test coverage.

Source: Coding guidelines

packages/envio/test/OnBlockSchema_test.res (1)

3-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove comments that narrate test purpose, callers, or refactor history. Retain only concise explanations of non-obvious invariants or surprising behavior.

  • packages/envio/test/OnBlockSchema_test.res#L3-L13: retain only the subtle two-stage validation invariant.
  • packages/envio/test/OnBlockSchema_test.res#L61-L62: remove the extractRange implementation pointer.
  • packages/envio/test/OnBlockSchema_test.res#L78-L83: keep the Some(undefined) rejection behavior, but remove old-behavior history.
  • packages/envio/test/ClientAddressFilter_test.res#L3-L5: remove the module-purpose summary.
  • packages/envio/test/ClientAddressFilter_test.res#L27-L29: remove the handler-reference narration.
  • packages/envio/test/helpers/MockIndexerConfig.res#L6-L6: remove the function-purpose summary.

As per coding guidelines, comments should explain only non-obvious constraints, subtle invariants, specific workarounds, or surprising behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/OnBlockSchema_test.res` around lines 3 - 13, Trim
comments at packages/envio/test/OnBlockSchema_test.res lines 3-13, 61-62, and
78-83, packages/envio/test/ClientAddressFilter_test.res lines 3-5 and 27-29, and
packages/envio/test/helpers/MockIndexerConfig.res line 6: retain only the
concise two-stage validation invariant in OnBlockSchema_test and the
Some(undefined) rejection behavior, while removing purpose summaries,
caller/handler references, extractRange pointers, and historical narration.

Source: Coding guidelines

packages/envio/test/Config_test.res (1)

229-270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add explicit types to every Utils.magic cast.

Annotate each cast itself with its concrete input type and Internal.eventParams output type rather than relying on the destination variable annotation.

As per coding guidelines, “When using Utils.magic for type casting in ReScript, always add explicit type annotations: value->(Utils.magic: inputType => outputType).”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/Config_test.res` around lines 229 - 270, Update every
Utils.magic cast in the affected tests to use explicit input and output type
annotations in the cast expression, following the inputType => outputType form.
For each testParams assignment, annotate the concrete object, tuple, or unit
input type and Internal.eventParams as the output, including the casts in the
existing event parameter tests.

Source: Coding guidelines

packages/envio/test/lib_tests/ChainState_materialize_test.res (1)

3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove comments that describe code structure or refactor history.

  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L3-L7: remove the helper-purpose/caller narration, or retain only a subtle invariant required by the unsafe representation.
  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L111-L113: remove the migration and former-caller history.
  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L188-L190: remove the migration and replacement history.
  • packages/envio/test/lib_tests/EffectCache_test.res#L3-L4: remove the module-purpose summary already conveyed by the describe name.

As per coding guidelines, “Don't write a comment that restates what the code already says” and “Never narrate the refactor itself.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res` around lines 3
- 7, Remove the structural and refactor-history comments at
packages/envio/test/lib_tests/ChainState_materialize_test.res lines 3-7,
111-113, and 188-190, retaining only an essential unsafe-representation
invariant if needed; remove the module-purpose summary at
packages/envio/test/lib_tests/EffectCache_test.res lines 3-4 because the
describe name already conveys it.

Source: Coding guidelines

packages/envio/test/Encoders_schema_test.res (1)

22-24: 🎯 Functional Correctness | 🔵 Trivial

Resolve the nullable encoding contract before locking it into the test.

The test says None encodes as null, but expects {}. Decide whether the wire format is {"optNumber":null} or omission, then align the title and expectation and remove the TODO. Would you like me to prepare either version?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/Encoders_schema_test.res` around lines 22 - 24, Resolve
the nullable encoding contract in the “encodes None as null” test and make the
assertion match the intended wire format: either include optNumber as null or
omit the field entirely. Update the test title to describe that behavior, adjust
the expected mock3raw value accordingly, and remove the TODO comment.
packages/envio/test/lib_tests/Rpc_Test.res (1)

4-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the commented-out alternative endpoint.

This stale assignment does not document a constraint; endpoint alternatives belong in configuration or commit history.

-  // let rpcUrl = "https://eth.rpc.hypersync.xyz"
   let rpcUrl = "https://eth.llamarpc.com"

As per coding guidelines, comments must not narrate prior alternatives or pointers to where values were defined.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/Rpc_Test.res` around lines 4 - 5, Remove the
commented-out alternative endpoint directly above the active rpcUrl assignment
in Rpc_Test; keep the active endpoint assignment unchanged and do not replace
the removed comment with another explanatory comment.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/envio/test/AddressLowercase_test.res`:
- Around line 14-17: Update the test “fromAddressLowercaseOrThrow returns
lowercase” to construct the address with a genuinely mixed-case hexadecimal
string, while keeping the expected output fully lowercase. This ensures
Address.Evm.fromAddressLowercaseOrThrow is verified to normalize casing rather
than merely return an already-lowercase input.

In `@packages/envio/test/Config_test.res`:
- Around line 151-176: Update the tests for evmBlockFieldSchema and
evmTransactionFieldSchema to collect the mapped values from each parsed field
and compare the resulting arrays directly with [1, 2, …, 27] and [1, 2, …, 32].
Replace the triangular-sum-only assertions while preserving the existing
iteration and parsing logic.

In `@packages/envio/test/HyperSync_test.res`:
- Line 46: Replace the Console.log call in the HyperSync test with a single
whole-value regression assertion that verifies the expected response shape or
normalized query result. Keep the assertion focused on the successful query
outcome and remove the logging entirely.

In `@packages/envio/test/HyperSyncClient_test.res`:
- Around line 3-6: Isolate live-test credential loading in both test modules: in
packages/envio/test/HyperSyncClient_test.res lines 3-6, gate the live suites
behind an explicit opt-in command or flag and resolve ENVIO_API_TOKEN only
inside that path; in packages/envio/test/HyperSync_test.res lines 3-6, move
token resolution into the skipped async test body so module import remains
credential-free.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res`:
- Around line 93-107: Update the materializePageItems no-op test to construct
`a` with existing inline transaction and block payloads, while keeping both
stores as `None`. Capture those payload identities before the call and assert
afterward that `rawTx(a)` and `rawBlock(a)` still contain the same values,
proving the no-op path preserves inline data.

In `@packages/envio/test/lib_tests/ColumnNameFormat_test.res`:
- Around line 245-291: Update the test block after creating the Postgres client
and storage in the Async.it callback to register guaranteed cleanup for both
resources. Ensure the schema is dropped and storage.close() executes when setup,
writes, reads, or assertions reject, and preserve cleanup ordering so
storage.close() still runs if the DROP SCHEMA operation fails.

In `@packages/envio/test/lib_tests/PackageJson_test.res`:
- Around line 5-6: In the package metadata test, remove the redundant untyped
Utils.EnvioPackage.value->Utils.magic truthiness assertion and retain the
version assertion as the single check. If Utils.magic is still required for the
version access, replace it with an explicitly typed cast rather than an untyped
assertion.

In `@packages/envio/test/RateLimit_test.res`:
- Around line 93-105: Strengthen the elapsed-time assertion in the parallel
getBlockHashes test to require both rate-limit windows, using an approximately
1000ms lower bound rather than 400ms. Keep the existing upper-bound tolerance
relative to elapsed and the deduplicated wall-clock behavior intact.

---

Nitpick comments:
In `@packages/envio/test/Config_test.res`:
- Around line 229-270: Update every Utils.magic cast in the affected tests to
use explicit input and output type annotations in the cast expression, following
the inputType => outputType form. For each testParams assignment, annotate the
concrete object, tuple, or unit input type and Internal.eventParams as the
output, including the casts in the existing event parameter tests.

In `@packages/envio/test/ConfigYaml_test.res`:
- Around line 33-64: Update the affected tests, including the case around
MockIndexerConfig.parseYaml and the ranges noted in the review, to gather all
relevant config, chain, source, contract, and event fields into one tuple or
record and verify them with a single toEqual assertion. Remove the individual
field-level expect calls while preserving the existing expected values and test
coverage.

In `@packages/envio/test/Encoders_schema_test.res`:
- Around line 22-24: Resolve the nullable encoding contract in the “encodes None
as null” test and make the assertion match the intended wire format: either
include optNumber as null or omit the field entirely. Update the test title to
describe that behavior, adjust the expected mock3raw value accordingly, and
remove the TODO comment.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res`:
- Around line 3-7: Remove the structural and refactor-history comments at
packages/envio/test/lib_tests/ChainState_materialize_test.res lines 3-7,
111-113, and 188-190, retaining only an essential unsafe-representation
invariant if needed; remove the module-purpose summary at
packages/envio/test/lib_tests/EffectCache_test.res lines 3-4 because the
describe name already conveys it.

In `@packages/envio/test/lib_tests/Rpc_Test.res`:
- Around line 4-5: Remove the commented-out alternative endpoint directly above
the active rpcUrl assignment in Rpc_Test; keep the active endpoint assignment
unchanged and do not replace the removed comment with another explanatory
comment.

In `@packages/envio/test/OnBlockSchema_test.res`:
- Around line 3-13: Trim comments at packages/envio/test/OnBlockSchema_test.res
lines 3-13, 61-62, and 78-83, packages/envio/test/ClientAddressFilter_test.res
lines 3-5 and 27-29, and packages/envio/test/helpers/MockIndexerConfig.res line
6: retain only the concise two-stage validation invariant in OnBlockSchema_test
and the Some(undefined) rejection behavior, while removing purpose summaries,
caller/handler references, extractRange pointers, and historical narration.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 23429f6c-fe35-42f0-bec5-854280136099

📥 Commits

Reviewing files that changed from the base of the PR and between 61274cd and d3144b3.

📒 Files selected for processing (37)
  • .github/workflows/build_and_verify.yml
  • package.json
  • packages/build-envio/src/build-artifact.ts
  • packages/envio/package.json
  • packages/envio/rescript.json
  • packages/envio/test/AddressLowercase_test.res
  • packages/envio/test/BlockStore_test.res
  • packages/envio/test/ClientAddressFilter_test.res
  • packages/envio/test/ConfigYaml_test.res
  • packages/envio/test/Config_test.res
  • packages/envio/test/Encoders_schema_test.res
  • packages/envio/test/EntityFilter_test.res
  • packages/envio/test/EventRouter_svm_test.res
  • packages/envio/test/HyperSyncClient_test.res
  • packages/envio/test/HyperSync_test.res
  • packages/envio/test/OnBlockSchema_test.res
  • packages/envio/test/RateLimit_test.res
  • packages/envio/test/ReorgDetection_test.res
  • packages/envio/test/SvmHyperSyncClient_test.res
  • packages/envio/test/SvmHyperSyncSource_test.res
  • packages/envio/test/TransactionStore_test.res
  • packages/envio/test/Utils_test.res
  • packages/envio/test/helpers/MockIndexerConfig.res
  • packages/envio/test/lib_tests/ChainState_materialize_test.res
  • packages/envio/test/lib_tests/ColumnNameFormat_test.res
  • packages/envio/test/lib_tests/ConfigEnvioInfo_test.res
  • packages/envio/test/lib_tests/EffectCache_test.res
  • packages/envio/test/lib_tests/Metrics_test.res
  • packages/envio/test/lib_tests/PackageJson_test.res
  • packages/envio/test/lib_tests/Rpc_Test.res
  • packages/envio/test/lib_tests/Throttler_test.res
  • packages/envio/test/setup.ts
  • packages/envio/vitest.config.ts
  • scenarios/test_codegen/test/IndexerState_test.res
  • scenarios/test_codegen/test/YamlConfigIndexer_test.res
  • scenarios/test_codegen/test/helpers/MockIndexer.res
  • scenarios/test_codegen/test/lib_tests/IndexerLoop_test.res

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 8

🧹 Nitpick comments (6)
packages/envio/test/ConfigYaml_test.res (1)

33-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Collapse each test to one whole-value assertion.

Build a tuple or record containing the relevant config, chain, source, contract, and event values, then compare it with one toEqual call.

As per coding guidelines, “Always use single assert to check the whole value instead of multiple asserts for every field.”

Also applies to: 66-98, 807-831, 881-895

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/ConfigYaml_test.res` around lines 33 - 64, Update the
affected tests, including the case around MockIndexerConfig.parseYaml and the
ranges noted in the review, to gather all relevant config, chain, source,
contract, and event fields into one tuple or record and verify them with a
single toEqual assertion. Remove the individual field-level expect calls while
preserving the existing expected values and test coverage.

Source: Coding guidelines

packages/envio/test/OnBlockSchema_test.res (1)

3-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove comments that narrate test purpose, callers, or refactor history. Retain only concise explanations of non-obvious invariants or surprising behavior.

  • packages/envio/test/OnBlockSchema_test.res#L3-L13: retain only the subtle two-stage validation invariant.
  • packages/envio/test/OnBlockSchema_test.res#L61-L62: remove the extractRange implementation pointer.
  • packages/envio/test/OnBlockSchema_test.res#L78-L83: keep the Some(undefined) rejection behavior, but remove old-behavior history.
  • packages/envio/test/ClientAddressFilter_test.res#L3-L5: remove the module-purpose summary.
  • packages/envio/test/ClientAddressFilter_test.res#L27-L29: remove the handler-reference narration.
  • packages/envio/test/helpers/MockIndexerConfig.res#L6-L6: remove the function-purpose summary.

As per coding guidelines, comments should explain only non-obvious constraints, subtle invariants, specific workarounds, or surprising behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/OnBlockSchema_test.res` around lines 3 - 13, Trim
comments at packages/envio/test/OnBlockSchema_test.res lines 3-13, 61-62, and
78-83, packages/envio/test/ClientAddressFilter_test.res lines 3-5 and 27-29, and
packages/envio/test/helpers/MockIndexerConfig.res line 6: retain only the
concise two-stage validation invariant in OnBlockSchema_test and the
Some(undefined) rejection behavior, while removing purpose summaries,
caller/handler references, extractRange pointers, and historical narration.

Source: Coding guidelines

packages/envio/test/Config_test.res (1)

229-270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add explicit types to every Utils.magic cast.

Annotate each cast itself with its concrete input type and Internal.eventParams output type rather than relying on the destination variable annotation.

As per coding guidelines, “When using Utils.magic for type casting in ReScript, always add explicit type annotations: value->(Utils.magic: inputType => outputType).”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/Config_test.res` around lines 229 - 270, Update every
Utils.magic cast in the affected tests to use explicit input and output type
annotations in the cast expression, following the inputType => outputType form.
For each testParams assignment, annotate the concrete object, tuple, or unit
input type and Internal.eventParams as the output, including the casts in the
existing event parameter tests.

Source: Coding guidelines

packages/envio/test/lib_tests/ChainState_materialize_test.res (1)

3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove comments that describe code structure or refactor history.

  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L3-L7: remove the helper-purpose/caller narration, or retain only a subtle invariant required by the unsafe representation.
  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L111-L113: remove the migration and former-caller history.
  • packages/envio/test/lib_tests/ChainState_materialize_test.res#L188-L190: remove the migration and replacement history.
  • packages/envio/test/lib_tests/EffectCache_test.res#L3-L4: remove the module-purpose summary already conveyed by the describe name.

As per coding guidelines, “Don't write a comment that restates what the code already says” and “Never narrate the refactor itself.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res` around lines 3
- 7, Remove the structural and refactor-history comments at
packages/envio/test/lib_tests/ChainState_materialize_test.res lines 3-7,
111-113, and 188-190, retaining only an essential unsafe-representation
invariant if needed; remove the module-purpose summary at
packages/envio/test/lib_tests/EffectCache_test.res lines 3-4 because the
describe name already conveys it.

Source: Coding guidelines

packages/envio/test/Encoders_schema_test.res (1)

22-24: 🎯 Functional Correctness | 🔵 Trivial

Resolve the nullable encoding contract before locking it into the test.

The test says None encodes as null, but expects {}. Decide whether the wire format is {"optNumber":null} or omission, then align the title and expectation and remove the TODO. Would you like me to prepare either version?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/Encoders_schema_test.res` around lines 22 - 24, Resolve
the nullable encoding contract in the “encodes None as null” test and make the
assertion match the intended wire format: either include optNumber as null or
omit the field entirely. Update the test title to describe that behavior, adjust
the expected mock3raw value accordingly, and remove the TODO comment.
packages/envio/test/lib_tests/Rpc_Test.res (1)

4-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the commented-out alternative endpoint.

This stale assignment does not document a constraint; endpoint alternatives belong in configuration or commit history.

-  // let rpcUrl = "https://eth.rpc.hypersync.xyz"
   let rpcUrl = "https://eth.llamarpc.com"

As per coding guidelines, comments must not narrate prior alternatives or pointers to where values were defined.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/Rpc_Test.res` around lines 4 - 5, Remove the
commented-out alternative endpoint directly above the active rpcUrl assignment
in Rpc_Test; keep the active endpoint assignment unchanged and do not replace
the removed comment with another explanatory comment.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/envio/test/AddressLowercase_test.res`:
- Around line 14-17: Update the test “fromAddressLowercaseOrThrow returns
lowercase” to construct the address with a genuinely mixed-case hexadecimal
string, while keeping the expected output fully lowercase. This ensures
Address.Evm.fromAddressLowercaseOrThrow is verified to normalize casing rather
than merely return an already-lowercase input.

In `@packages/envio/test/Config_test.res`:
- Around line 151-176: Update the tests for evmBlockFieldSchema and
evmTransactionFieldSchema to collect the mapped values from each parsed field
and compare the resulting arrays directly with [1, 2, …, 27] and [1, 2, …, 32].
Replace the triangular-sum-only assertions while preserving the existing
iteration and parsing logic.

In `@packages/envio/test/HyperSync_test.res`:
- Line 46: Replace the Console.log call in the HyperSync test with a single
whole-value regression assertion that verifies the expected response shape or
normalized query result. Keep the assertion focused on the successful query
outcome and remove the logging entirely.

In `@packages/envio/test/HyperSyncClient_test.res`:
- Around line 3-6: Isolate live-test credential loading in both test modules: in
packages/envio/test/HyperSyncClient_test.res lines 3-6, gate the live suites
behind an explicit opt-in command or flag and resolve ENVIO_API_TOKEN only
inside that path; in packages/envio/test/HyperSync_test.res lines 3-6, move
token resolution into the skipped async test body so module import remains
credential-free.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res`:
- Around line 93-107: Update the materializePageItems no-op test to construct
`a` with existing inline transaction and block payloads, while keeping both
stores as `None`. Capture those payload identities before the call and assert
afterward that `rawTx(a)` and `rawBlock(a)` still contain the same values,
proving the no-op path preserves inline data.

In `@packages/envio/test/lib_tests/ColumnNameFormat_test.res`:
- Around line 245-291: Update the test block after creating the Postgres client
and storage in the Async.it callback to register guaranteed cleanup for both
resources. Ensure the schema is dropped and storage.close() executes when setup,
writes, reads, or assertions reject, and preserve cleanup ordering so
storage.close() still runs if the DROP SCHEMA operation fails.

In `@packages/envio/test/lib_tests/PackageJson_test.res`:
- Around line 5-6: In the package metadata test, remove the redundant untyped
Utils.EnvioPackage.value->Utils.magic truthiness assertion and retain the
version assertion as the single check. If Utils.magic is still required for the
version access, replace it with an explicitly typed cast rather than an untyped
assertion.

In `@packages/envio/test/RateLimit_test.res`:
- Around line 93-105: Strengthen the elapsed-time assertion in the parallel
getBlockHashes test to require both rate-limit windows, using an approximately
1000ms lower bound rather than 400ms. Keep the existing upper-bound tolerance
relative to elapsed and the deduplicated wall-clock behavior intact.

---

Nitpick comments:
In `@packages/envio/test/Config_test.res`:
- Around line 229-270: Update every Utils.magic cast in the affected tests to
use explicit input and output type annotations in the cast expression, following
the inputType => outputType form. For each testParams assignment, annotate the
concrete object, tuple, or unit input type and Internal.eventParams as the
output, including the casts in the existing event parameter tests.

In `@packages/envio/test/ConfigYaml_test.res`:
- Around line 33-64: Update the affected tests, including the case around
MockIndexerConfig.parseYaml and the ranges noted in the review, to gather all
relevant config, chain, source, contract, and event fields into one tuple or
record and verify them with a single toEqual assertion. Remove the individual
field-level expect calls while preserving the existing expected values and test
coverage.

In `@packages/envio/test/Encoders_schema_test.res`:
- Around line 22-24: Resolve the nullable encoding contract in the “encodes None
as null” test and make the assertion match the intended wire format: either
include optNumber as null or omit the field entirely. Update the test title to
describe that behavior, adjust the expected mock3raw value accordingly, and
remove the TODO comment.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res`:
- Around line 3-7: Remove the structural and refactor-history comments at
packages/envio/test/lib_tests/ChainState_materialize_test.res lines 3-7,
111-113, and 188-190, retaining only an essential unsafe-representation
invariant if needed; remove the module-purpose summary at
packages/envio/test/lib_tests/EffectCache_test.res lines 3-4 because the
describe name already conveys it.

In `@packages/envio/test/lib_tests/Rpc_Test.res`:
- Around line 4-5: Remove the commented-out alternative endpoint directly above
the active rpcUrl assignment in Rpc_Test; keep the active endpoint assignment
unchanged and do not replace the removed comment with another explanatory
comment.

In `@packages/envio/test/OnBlockSchema_test.res`:
- Around line 3-13: Trim comments at packages/envio/test/OnBlockSchema_test.res
lines 3-13, 61-62, and 78-83, packages/envio/test/ClientAddressFilter_test.res
lines 3-5 and 27-29, and packages/envio/test/helpers/MockIndexerConfig.res line
6: retain only the concise two-stage validation invariant in OnBlockSchema_test
and the Some(undefined) rejection behavior, while removing purpose summaries,
caller/handler references, extractRange pointers, and historical narration.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 23429f6c-fe35-42f0-bec5-854280136099

📥 Commits

Reviewing files that changed from the base of the PR and between 61274cd and d3144b3.

📒 Files selected for processing (37)
  • .github/workflows/build_and_verify.yml
  • package.json
  • packages/build-envio/src/build-artifact.ts
  • packages/envio/package.json
  • packages/envio/rescript.json
  • packages/envio/test/AddressLowercase_test.res
  • packages/envio/test/BlockStore_test.res
  • packages/envio/test/ClientAddressFilter_test.res
  • packages/envio/test/ConfigYaml_test.res
  • packages/envio/test/Config_test.res
  • packages/envio/test/Encoders_schema_test.res
  • packages/envio/test/EntityFilter_test.res
  • packages/envio/test/EventRouter_svm_test.res
  • packages/envio/test/HyperSyncClient_test.res
  • packages/envio/test/HyperSync_test.res
  • packages/envio/test/OnBlockSchema_test.res
  • packages/envio/test/RateLimit_test.res
  • packages/envio/test/ReorgDetection_test.res
  • packages/envio/test/SvmHyperSyncClient_test.res
  • packages/envio/test/SvmHyperSyncSource_test.res
  • packages/envio/test/TransactionStore_test.res
  • packages/envio/test/Utils_test.res
  • packages/envio/test/helpers/MockIndexerConfig.res
  • packages/envio/test/lib_tests/ChainState_materialize_test.res
  • packages/envio/test/lib_tests/ColumnNameFormat_test.res
  • packages/envio/test/lib_tests/ConfigEnvioInfo_test.res
  • packages/envio/test/lib_tests/EffectCache_test.res
  • packages/envio/test/lib_tests/Metrics_test.res
  • packages/envio/test/lib_tests/PackageJson_test.res
  • packages/envio/test/lib_tests/Rpc_Test.res
  • packages/envio/test/lib_tests/Throttler_test.res
  • packages/envio/test/setup.ts
  • packages/envio/vitest.config.ts
  • scenarios/test_codegen/test/IndexerState_test.res
  • scenarios/test_codegen/test/YamlConfigIndexer_test.res
  • scenarios/test_codegen/test/helpers/MockIndexer.res
  • scenarios/test_codegen/test/lib_tests/IndexerLoop_test.res
🛑 Comments failed to post (8)
packages/envio/test/AddressLowercase_test.res (1)

14-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use a genuinely mixed-case address.

The lowercase input does not verify normalization and would pass if the function returned its input unchanged.

Proposed fix
-    let mixed = Address.Evm.fromStringOrThrow("0x2c169dfe5fbba12957bdd0ba47d9cedbfe260ca7")
+    let mixed = Address.Evm.fromStringOrThrow("0x2C169DFe5fBbA12957Bdd0Ba47d9CEDbFE260CA7")
📝 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.

  it("fromAddressLowercaseOrThrow returns lowercase", t => {
    let mixed = Address.Evm.fromStringOrThrow("0x2C169DFe5fBbA12957Bdd0Ba47d9CEDbFE260CA7")
    let out = mixed->Address.Evm.fromAddressLowercaseOrThrow->Address.toString
    t.expect(out).toBe("0x2c169dfe5fbba12957bdd0ba47d9cedbfe260ca7")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/AddressLowercase_test.res` around lines 14 - 17, Update
the test “fromAddressLowercaseOrThrow returns lowercase” to construct the
address with a genuinely mixed-case hexadecimal string, while keeping the
expected output fully lowercase. This ensures
Address.Evm.fromAddressLowercaseOrThrow is verified to normalize casing rather
than merely return an already-lowercase input.
packages/envio/test/Config_test.res (1)

151-176: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the complete mapped variant arrays, not only their sums.

A duplicate/omission combination can preserve the triangular sum. Compare the mapped values with [1, 2, …, 27] and [1, 2, …, 32] so every variant is proven present exactly once.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/Config_test.res` around lines 151 - 176, Update the tests
for evmBlockFieldSchema and evmTransactionFieldSchema to collect the mapped
values from each parsed field and compare the resulting arrays directly with [1,
2, …, 27] and [1, 2, …, 32]. Replace the triangular-sum-only assertions while
preserving the existing iteration and parsing logic.
packages/envio/test/HyperSync_test.res (1)

46-46: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Replace the log with a regression assertion.

This test currently verifies nothing once the query succeeds. Assert the expected response shape or normalized result using one whole-value assertion.

As per coding guidelines, tests must never log and must use assertions for verification.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/HyperSync_test.res` at line 46, Replace the Console.log
call in the HyperSync test with a single whole-value regression assertion that
verifies the expected response shape or normalized query result. Keep the
assertion focused on the successful query outcome and remove the logging
entirely.

Source: Coding guidelines

packages/envio/test/HyperSyncClient_test.res (1)

3-6: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Isolate live-test credentials from default test discovery.

Both modules resolve ENVIO_API_TOKEN at import time, making ordinary package tests dependent on live-test credentials before suite-level skipping or gating can apply.

  • packages/envio/test/HyperSyncClient_test.res#L3-L6: place the live suites behind an explicit opt-in command/flag and resolve the token only inside that path.
  • packages/envio/test/HyperSync_test.res#L3-L6: move token resolution into the skipped async test body so importing the module remains credential-free.
📍 Affects 2 files
  • packages/envio/test/HyperSyncClient_test.res#L3-L6 (this comment)
  • packages/envio/test/HyperSync_test.res#L3-L6
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/HyperSyncClient_test.res` around lines 3 - 6, Isolate
live-test credential loading in both test modules: in
packages/envio/test/HyperSyncClient_test.res lines 3-6, gate the live suites
behind an explicit opt-in command or flag and resolve ENVIO_API_TOKEN only
inside that path; in packages/envio/test/HyperSync_test.res lines 3-6, move
token resolution into the skipped async test body so module import remains
credential-free.
packages/envio/test/lib_tests/ChainState_materialize_test.res (1)

93-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Exercise the no-op path with existing inline payloads.

a contains neither payload, so this only proves absent data stays absent. Provide inline transaction and block values and assert their identities remain unchanged.

Proposed test adjustment
-    let a = makeItem(~blockNumber=1, ~transactionIndex=1, ~transactionMask=2., ~blockMask=2.)
+    let inlineTx = {"hash": "0xinline"}->(Utils.magic: {..} => Internal.eventTransaction)
+    let inlineBlock = {"number": 1}->(Utils.magic: {..} => Internal.eventBlock)
+    let a = makeItem(
+      ~blockNumber=1,
+      ~transactionIndex=1,
+      ~inlineTransaction=inlineTx,
+      ~inlineBlock,
+    )
...
-      "tx": rawTx(a)->Nullable.toOption,
-      "block": rawBlock(a)->Nullable.toOption,
+      "txUntouched": rawTx(a) === inlineTx->Nullable.make,
+      "blockUntouched": rawBlock(a) === inlineBlock->Nullable.make,
...
-      "tx": None,
-      "block": None,
+      "txUntouched": true,
+      "blockUntouched": true,
📝 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.

  Async.it("materializePageItems is a no-op for None pages (RPC/Fuel/Simulate)", async t => {
    let inlineTx = {"hash": "0xinline"}->(Utils.magic: {..} => Internal.eventTransaction)
    let inlineBlock = {"number": 1}->(Utils.magic: {..} => Internal.eventBlock)
    let a = makeItem(
      ~blockNumber=1,
      ~transactionIndex=1,
      ~inlineTransaction=inlineTx,
      ~inlineBlock,
    )
    await ChainState.materializePageItems(
      ~items=[a],
      ~transactionStore=None,
      ~blockStore=None,
      ~ecosystem=Ecosystem.Fuel,
    )
    t.expect({
      "txUntouched": rawTx(a) === inlineTx->Nullable.make,
      "blockUntouched": rawBlock(a) === inlineBlock->Nullable.make,
    }).toEqual({
      "txUntouched": true,
      "blockUntouched": true,
    })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/ChainState_materialize_test.res` around lines
93 - 107, Update the materializePageItems no-op test to construct `a` with
existing inline transaction and block payloads, while keeping both stores as
`None`. Capture those payload identities before the call and assert afterward
that `rawTx(a)` and `rawBlock(a)` still contain the same values, proving the
no-op path preserves inline data.
packages/envio/test/lib_tests/ColumnNameFormat_test.res (1)

245-291: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guarantee database cleanup when setup or assertions fail.

Any rejected operation before Lines 290-291 leaves the connection open and schema behind, potentially contaminating later tests. Establish a guaranteed cleanup path immediately after resource creation, and ensure storage.close() still runs if dropping the schema fails.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/ColumnNameFormat_test.res` around lines 245 -
291, Update the test block after creating the Postgres client and storage in the
Async.it callback to register guaranteed cleanup for both resources. Ensure the
schema is dropped and storage.close() executes when setup, writes, reads, or
assertions reject, and preserve cleanup ordering so storage.close() still runs
if the DROP SCHEMA operation fails.
packages/envio/test/lib_tests/PackageJson_test.res (1)

5-6: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove the redundant untyped Utils.magic assertion.

value.version already covers the needed check here; keep a single assertion and, if the cast remains necessary, use an explicitly typed Utils.magic cast.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/lib_tests/PackageJson_test.res` around lines 5 - 6, In
the package metadata test, remove the redundant untyped
Utils.EnvioPackage.value->Utils.magic truthiness assertion and retain the
version assertion as the single check. If Utils.magic is still required for the
version access, replace it with an explicitly typed cast rather than an untyped
assertion.
packages/envio/test/RateLimit_test.res (1)

93-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that both rate-limit windows are accounted for.

The current > 400ms bound still passes if one expected 500ms window is omitted, so it does not verify the documented approximately 1000ms deduplicated total.

-      t.expect(rateLimitTime > 400.0 && rateLimitTime < elapsed +. 100.0).toEqual(true)
+      t.expect(rateLimitTime > 900.0 && rateLimitTime < elapsed +. 100.0).toEqual(true)
📝 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.

      // Two parallel calls — each hits 2 rate limits at ~500ms each.
      // Sequential accounting would yield ~4 * 500ms = 2000ms; the dedup'd
      // wall-clock total should be roughly half that (~1000ms).
      let start = Date.now()
      let _ =
        await [
          sourceManager->SourceManager.getBlockHashes(~blockNumbers=[1], ~isRealtime=false),
          sourceManager->SourceManager.getBlockHashes(~blockNumbers=[2], ~isRealtime=false),
        ]->Promise.all
      let elapsed = Date.now() -. start

      let rateLimitTime = sourceManager->SourceManager.getRateLimitTimeMs
      t.expect(rateLimitTime > 900.0 && rateLimitTime < elapsed +. 100.0).toEqual(true)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/test/RateLimit_test.res` around lines 93 - 105, Strengthen the
elapsed-time assertion in the parallel getBlockHashes test to require both
rate-limit windows, using an approximately 1000ms lower bound rather than 400ms.
Keep the existing upper-bound tolerance relative to elapsed and the deduplicated
wall-clock behavior intact.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/envio/vitest.config.ts (1)

13-13: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove passWithNoTests from the CI test path
vitest run will succeed with zero matched tests, so a bad include glob or missing generated .res.mjs files can make this package’s test job go green without executing anything. Drop this option, or gate it off outside CI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/envio/vitest.config.ts` at line 13, Remove the passWithNoTests
option from the Vitest configuration so the CI vitest run fails when no tests
are matched, preserving test-path validation for the package.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/envio/vitest.config.ts`:
- Line 13: Remove the passWithNoTests option from the Vitest configuration so
the CI vitest run fails when no tests are matched, preserving test-path
validation for the package.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df322730-e42f-4f55-9ef5-d28984a768ea

📥 Commits

Reviewing files that changed from the base of the PR and between d3144b3 and 4edf6ed.

📒 Files selected for processing (1)
  • packages/envio/vitest.config.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9812a779fc

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

| None => Config.load()
}
let indexerState = IndexerState.make(
~config,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Build persistence from the supplied config

When callers use the newly added ~config with MockIndexer.InMemoryStore.make, this still passes a persistence object cached from Config.load(). IndexerState.make derives its entity tables from persistence.allEntities, so a custom YAML-only entity is absent and setEntity (or any lookup of that entity) raises UndefinedEntity; construct the persistence with this local config rather than defaultPersistence().

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29200ef9fc

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +483 to +484
HandlerRegister.startRegistration(~config)
HandlerRegister.finishRegistration(~config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear cached handlers for custom configs

When a custom YAML config reuses a contract/event name that was already registered earlier in the same Vitest worker (for example after a default MockIndexer run loaded generated handlers), this path still lets HandlerRegister.finishRegistration reuse the process-wide handler cache via hasRegistration. That attaches stale generated handlers/registrations to the supplied config even though this branch intentionally avoids handler files, so mock-source fetch partitions and dispatch can be driven by the wrong project; clear or ignore the cached registry for the custom-config path before finishing registration.

Useful? React with 👍 / 👎.

Async.it(
"runs the indexer loop with a config parsed from user YAML instead of the generated one",
async t => {
let parsed = MockIndexerConfig.parseYaml(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the YAML parser helper with the scenario tests

In CI I checked the artifact install path: .pnpmfile.cjs redirects scenarios/test_codegen's envio dependency to the built artifact, and this commit strips test from that artifact's rescript.json/doesn't ship packages/envio/test. In that environment this new scenario test resolves MockIndexerConfig only from envio's test sources after the scenario-local helper was deleted, so pnpm test in scenarios/test_codegen fails to compile with an unbound module; keep a scenario-local helper or move it into shipped source.

Useful? React with 👍 / 👎.

Comment on lines +365 to +366
~contractName=switch chainConfig.contracts->Array.get(0) {
| Some(contract) => contract.name

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the selected contract start block

When the supplied YAML config gives this first contract its own start_block, the only registration created for the custom-config mock source still has startBlock: None, so FetchState derives the contract partition from the chain start block and the mock source queries blocks before that contract should be active. Pass the selected contract's startBlock into makeMockSourceRegistration (and onto the registration) so custom-config tests with per-contract start blocks don't process out-of-range events.

Useful? React with 👍 / 👎.

YamlConfigIndexer_test referenced MockIndexerConfig, which now lives only
in envio's test dir and is excluded from the published npm artifact that
scenarios build against in CI. Inline the parse via the public
Core.parseConfigYaml + Config.fromPublic instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh
…cating

test_codegen's acceptance test needs the YAML parse helper that lives in
the envio package. It compiled locally (workspace symlink exposes envio's
lib/ocaml) but broke in CI, where scenarios build against the published
artifact that excludes the test dir.

Move MockIndexerConfig into a new packages/envio/testing dir: vitest-free
ReScript helpers that ship in the artifact and stay in the published
rescript.json sources, so dependent suites compile against them in CI.
The test dir (vitest suites) stays stripped. The helper is not part of
the user-facing JS API (index.js), keeping it internal. Kept the module
name to avoid colliding with test_codegen's own MockConfig.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 89f4be462e

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

"src",
// Internal ReScript test helpers (vitest-free) that dependent test suites
// compile against. Not part of the user-facing JS API (index.js).
"testing",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add testing to artifact allow-lists

When the build-envio-package workflow runs, node packages/build-envio/src/build-artifact.ts ... --out envio-dist now copies this top-level testing dir, and the very next step invokes verify-artifact.ts envio-dist; that verifier's REQUIRED_FILES allow-list still omits testing, so CI fails with Unexpected file in artifact: testing. Fresh evidence beyond the earlier helper comment: npm pack --dry-run on the built artifact also produced zero testing/ files because the generated package.json inherits a files allow-list that omits it, so relaxing the verifier alone still leaves artifact consumers unable to compile MockIndexerConfig.

Useful? React with 👍 / 👎.

Comment on lines +365 to +367
~contractName=switch chainConfig.contracts->Array.get(0) {
| Some(contract) => contract.name
| None => "MockContract"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Choose an addressed contract for mock registration

When a supplied YAML chain lists an address-less/dynamic contract first and an addressed contract later, this binds the synthetic mock event to the address-less contract. FetchState.make only creates address-dependent partitions from configured addresses whose contract name appears in the registrations, so the mock source gets no partition for that chain and the indexer either throws Invalid configuration: Nothing to fetch or never queries the custom source; select a contract with configured addresses (or make the synthetic registration wildcard) instead of always using element 0.

Useful? React with 👍 / 👎.

Replace the test/testing split inside the envio package with a separate
private workspace package, envio-tests, that depends on envio. It holds
all the (non-generated-code) test suites and the MockIndexerConfig helper.

- envio ships pure runtime: no test/testing dirs, no vitest devDep.
  build-artifact.ts drops stripTestSources and the testing PUBLISH entry.
- envio-tests runs its vitest suite against the envio artifact in CI (via
  the pnpmfile redirect), so it loads the NAPI addon through the artifact's
  platform dependency like scenarios do — no NODE_PATH hack.
- test_codegen depends on envio-tests to reuse MockIndexerConfig in its
  YAML-driven acceptance test, instead of duplicating the parse helper.
- CI 'envio package tests' step becomes 'envio-tests'; root test script
  and workspace wiring updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh
Main replaced the prom-client-based metrics with a custom Metrics module,
so nothing imports prom-client at runtime anymore.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fb6Yyif9GZG1DWAiCER4Gh
@DZakh
DZakh merged commit 003d0a8 into main Jul 21, 2026
8 checks passed
@DZakh
DZakh deleted the claude/configyaml-mockindexer-unify-xmdo4l branch July 21, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants