Skip to content

📝 docs: fix missing and incorrect JSDoc across core public API - #2087

Open
roninjin10 wants to merge 1 commit into
mainfrom
quality/jsdoc-core
Open

📝 docs: fix missing and incorrect JSDoc across core public API#2087
roninjin10 wants to merge 1 commit into
mainfrom
quality/jsdoc-core

Conversation

@roninjin10

@roninjin10 roninjin10 commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Audited the public API surface of core packages (avoiding packages/actions, packages/errors, packages/node, packages/server, packages/memory-client which have in-flight PRs) and fixed missing and actively wrong JSDoc.

Actively wrong JSDoc (findings)

  • @tevm/effectfileExists, logAllErrors, and createRequireEffect examples imported from a nonexistent package @eth-optimism/config (stale copy-paste). fileExists's example also awaited an Effect directly, which doesn't run it. resolve.js had a stray backtick breaking the docblock and four-backtick code fences.
  • @tevm/jsonrpccreateJsonRpcFetcher's example called a fetchJsonRpc(url, params) function that does not exist; its @returns claimed to return "the result field from the JSON-RPC response" when it actually returns a JsonRpcClient object.
  • @tevm/http-clientcreateHttpClient had a completely empty @example block.
  • @tevm/commoncreateCommon examples used { ... } elisions that can't be pasted and run.
  • @tevm/utilsrecoverMessageAddress and verifyMessage examples used placeholder r: 0x... / s: 0x... values; replaced with the real test vectors from signature.spec.ts (message 'Hello world', expected address 0xED54a7C1d8634BB589f24Bb7F05a5554b36F9618).

Missing JSDoc added

  • @tevm/utils: invariant (no JSDoc at all), PREFUNDED_PRIVATE_KEYS / PREFUNDED_PUBLIC_KEYS / PREFUNDED_ACCOUNTS / PREFUNDED_SEED (no JSDoc)
  • @tevm/logger: createLogger gained @example + @throws
  • @tevm/sync-storage-persister: createSyncStoragePersister gained @example + @throws (+ syncronous typo fix); noopPersister gained @example
  • @tevm/common: createMockKzg gained a @returns description

All new examples are complete (real imports, no elisions) and based on the packages' own test files.

Verification

  • tsc --noEmit passes in all 7 touched packages (after nx run-many --targets=build:types for the 7 projects + 72 dependency tasks)
  • vitest run results:
    • @tevm/effect: 17 passed
    • @tevm/logger: 1 passed
    • @tevm/jsonrpc: 7 passed
    • @tevm/sync-storage-persister: 15 passed
    • @tevm/common: 20 passed
    • @tevm/utils: 59 passed, 1 skipped
    • @tevm/http-client: suite is describe.skip upstream (8 skipped, 1 todo) — unchanged by this PR
  • biome check / biome format: clean on all touched files
LANE: Fix missing and wrong JSDoc across the core public API. Audit and improve JSDoc across evmts/tevm's public API. The repo convention is explicit: when JSDoc is missing you add it, and existing JSDoc may be WRONG so fix it. Avoid packages/actions, packages/errors, packages/node, packages/server and packages/memory-client — in-flight PRs touch them.

Summary by CodeRabbit

  • Documentation
    • Improved public API documentation across multiple packages.
    • Added complete, runnable examples for effects, HTTP clients, JSON-RPC, logging, persistence, utilities, and account helpers.
    • Corrected examples, imports, parameter descriptions, return values, and error behavior.
    • Documented prefunded account data and clarified that the associated private keys are publicly known.
    • Updated package patch versions.

- @tevm/effect: examples imported from nonexistent '@eth-optimism/config',
  fix to '@tevm/effect' and make examples runnable (runPromise, tapError imports);
  fix stray backtick and four-backtick fences; fix 'wrpper' typo
- @tevm/jsonrpc: createJsonRpcFetcher example called nonexistent fetchJsonRpc;
  fix @returns description (returns JsonRpcClient, not the result field)
- @tevm/http-client: createHttpClient had an empty @example block; add working example
- @tevm/logger: createLogger add @example and @throws
- @tevm/sync-storage-persister: createSyncStoragePersister add @example/@throws;
  noopPersister add @example; fix 'syncronous' typo
- @tevm/common: createCommon examples used '...' elisions; replace with complete
  runnable examples based on createCommon.spec.ts; createMockKzg add @returns description
- @tevm/utils: invariant had no JSDoc; prefundedAccounts constants had no JSDoc;
  signature.js recoverMessageAddress/verifyMessage examples had elided '0x...' r/s
  values replaced with the real test vectors from signature.spec.ts

<prompt>
LANE: Fix missing and wrong JSDoc across the core public API. Audit and improve JSDoc
across evmts/tevm's public API. When JSDoc is missing you add it, and existing JSDoc
may be WRONG so fix it. Avoid packages/actions, packages/errors, packages/node,
packages/server and packages/memory-client (in-flight PRs).
</prompt>
@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cd76603

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package or glob expression "@evmts/*" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch

@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
node Ready Ready Preview Jul 30, 2026 2:23am
tevm-monorepo-app Ready Ready Preview Jul 30, 2026 2:23am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
tevm-monorepo-tevm Ignored Ignored Jul 30, 2026 2:23am

Request Review

@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

JSDoc across multiple @tevm/* packages now contains corrected imports, runnable examples, and expanded parameter, return, and error descriptions. fileExists also adds debug logging, while a changeset records patch releases.

Changes

Public API documentation

Layer / File(s) Summary
Effect examples and filesystem logging
packages/effect/src/*
Effect examples now use current imports and runPromise; parseJson and resolver documentation is corrected, and fileExists logs checks and results.
Core API contracts and examples
packages/common/src/*, packages/http-client/src/*, packages/jsonrpc/src/*, packages/logger/src/*
Examples and JSDoc describe corrected configuration, JSON-RPC response shapes, logger usage, HTTP client usage, and mock KZG behavior.
Persistence and utility documentation
packages/sync-storage-persister/src/*, packages/utils/src/*
Persistence, invariant, prefunded account, and signature documentation is expanded or corrected with concrete examples.
Release metadata
.changeset/tidy-geese-speak.md
Patch releases and the documentation-focused corrections are recorded.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the JSDoc documentation fixes across the core public API.
Description check ✅ Passed The description covers the change summary and verification results; only the optional Additional Information section is missing.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch quality/jsdoc-core

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.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


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.

❤️ Share

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: 6

Caution

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

⚠️ Outside diff range comments (1)
packages/effect/src/fileExists.js (1)

19-29: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Avoid logging the raw caller-supplied path.

fileExists interpolates path into two logDebug calls. If these debug logs can be emitted by callers, filesystem details may be exposed and raw controls/newlines could be written to the configured logger. Keep these debug messages path-free or sanitize/redact path before logging.

🤖 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/effect/src/fileExists.js` around lines 19 - 29, Update fileExists so
neither logDebug call interpolates the raw caller-supplied path; keep the debug
messages path-free or use the existing sanitization/redaction mechanism before
logging, while preserving the access result behavior.
🤖 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/common/src/createCommon.js`:
- Around line 25-33: Update the JSDoc examples in createCommon to be
self-contained: add the createCommon import to the first example, and change the
second and third examples to import createCommon from `@tevm/common` and mainnet
from viem/chains. Keep each example’s existing usage and options unchanged.

In `@packages/effect/src/parseJson.js`:
- Around line 26-34: Update the JSDoc for parseJson to describe failures as
invalid JSONC according to the parser’s supported format, including allowed
comments and trailing commas, rather than strictly invalid JSON. Clarify that
ParseJsonError is raised when the returned Effect is executed, while parseJson
itself returns an Effect.

In `@packages/effect/src/resolve.js`:
- Around line 38-44: Execute both Effect examples in
packages/effect/src/resolve.js: update the resolveSync example around
resolveSync and tap to run through the Effect runtime, and update the
resolveAsync example around resolveAsync and tap to use async runtime execution
with await. Ensure both examples actually execute so console.log is invoked.

In `@packages/jsonrpc/src/createJsonRpcFetcher.js`:
- Around line 7-25: Update the createJsonRpcFetcher usage example so it checks
the full response from fetcher.request for an RPC error before accessing result,
then guards against a null or missing block before printing block.number.
Preserve the existing eth_getBlockByNumber request and successful-response
behavior while demonstrating safe handling of both failure cases.

In `@packages/sync-storage-persister/src/createSyncStoragePersister.js`:
- Around line 15-38: Update the example around createSyncStoragePersister so
restoreState() runs only after the throttled persistTevmState() operation has
completed, and ensure removePersistedState() does not cancel the pending save.
Use an awaited delay or set throttleTime to 0, while keeping the example
executable and deterministic.

In `@packages/sync-storage-persister/src/noopPersister.js`:
- Around line 7-14: Update the noopPersister JSDoc example to pass undefined or
a complete SerializableTevmState to persistTevmState instead of {}. Keep the
example’s demonstrated no-op behavior and restoreState usage unchanged.

---

Outside diff comments:
In `@packages/effect/src/fileExists.js`:
- Around line 19-29: Update fileExists so neither logDebug call interpolates the
raw caller-supplied path; keep the debug messages path-free or use the existing
sanitization/redaction mechanism before logging, while preserving the access
result behavior.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ba04c2f-c2fc-435e-ad4d-b0ae1e670598

📥 Commits

Reviewing files that changed from the base of the PR and between c1ad08e and cd76603.

📒 Files selected for processing (16)
  • .changeset/tidy-geese-speak.md
  • packages/common/src/createCommon.js
  • packages/common/src/createMockKzg.js
  • packages/effect/src/createRequireEffect.js
  • packages/effect/src/fileExists.js
  • packages/effect/src/logAllErrors.js
  • packages/effect/src/parseJson.js
  • packages/effect/src/resolve.js
  • packages/http-client/src/createHttpClient.js
  • packages/jsonrpc/src/createJsonRpcFetcher.js
  • packages/logger/src/createLogger.js
  • packages/sync-storage-persister/src/createSyncStoragePersister.js
  • packages/sync-storage-persister/src/noopPersister.js
  • packages/utils/src/invariant.ts
  • packages/utils/src/prefundedAccounts.ts
  • packages/utils/src/signature.js

Comment on lines +25 to 33
* import { mainnet } from 'viem/chains'
*
* const common = createCommon({
* customCrypto: {},
* loggingLevel: 'debug',
* hardfork: 'london',
* eips: [420],
* id: 69,
* name: 'MyChain',
* ...
* ...mainnet,
* customCrypto: {},
* loggingLevel: 'debug',
* hardfork: 'london',
* eips: [420],
* })

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the examples self-contained and use the published package imports.

The first example uses createCommon without importing it. The second and third examples import from tevm/common; they should import createCommon from @tevm/common and mainnet from viem/chains.

As per coding guidelines, “We always make the examples working examples” and “Include imports in examples in JSDoc.”

Proposed fix
-import { mainnet } from 'viem/chains'
+import { createCommon } from '`@tevm/common`'
+import { mainnet } from 'viem/chains'

-import { createCommon, mainnet } from 'tevm/common'
+import { createCommon } from '`@tevm/common`'
+import { mainnet } from 'viem/chains'

Also applies to: 37-50

🤖 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/common/src/createCommon.js` around lines 25 - 33, Update the JSDoc
examples in createCommon to be self-contained: add the createCommon import to
the first example, and change the second and third examples to import
createCommon from `@tevm/common` and mainnet from viem/chains. Keep each example’s
existing usage and options unchanged.

Source: Coding guidelines

Comment on lines +26 to +34
* @throws {ParseJsonError} when the json string is not valid json
* @example
* ```ts
* import { runPromise } from 'effect/Effect'
* import { parseJson } from '@tevm/effect'
*
* const jsonEffect = parseJson('{ "compilerOptions": { "plugins": [{ "name": "@tevm/ts-plugin" }] } }')
* ````
* const json = await runPromise(jsonEffect)
* ```

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg '(^|/)parseJson\.js$|(^|/)parseJson\.(js|ts)$' || true

echo
echo "parseJson.js excerpt:"
if [ -f packages/effect/src/parseJson.js ]; then
  nl -ba packages/effect/src/parseJson.js | sed -n '1,140p'
fi

echo
echo "ParseJsonError usages:"
rg -n "ParseJsonError|parseJson" packages/effect/src -S || true

echo
echo "Imports related to parse-jsonc/jsonc:"
rg -n "parseJsonc|jsonc|comments|trailing|ParseJsonError" . -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200

Repository: evmts/tevm

Length of output: 252


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="packages/effect/src/parseJson.js"

echo "parseJson.js excerpt:"
awk '{printf "%6d  %s\n", NR, $0}' "$file" | sed -n '1,160p'

echo
echo "ParseJsonError/parseJson occurrences:"
rg -n "ParseJsonError|function parseJson|const parseJson|parseJson\\(" packages/effect/src -S || true

echo
echo "JSON parser config references:"
rg -n "parseJsonc|jsonc|trailing|comments|JSONC|ParseJsonError" . -S --glob '!node_modules' --glob '!dist' --glob '!build' | head -200 || true

Repository: evmts/tevm

Length of output: 30049


Describe JSONC parsing failures precisely.

parseJson delegates to jsonc-parser with comments and trailing commas allowed, so @throws {ParseJsonError} when the json string is not valid json misstates the accepted format. Also clarify that the failure occurs when the returned Effect is run, because parseJson itself returns an Effect.

Proposed wording
- * `@throws` {ParseJsonError} when the json string is not valid json
+ * `@throws` {ParseJsonError} when running the returned Effect fails to parse
+ * `jsonStr` with the configured JSONC options
📝 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.

Suggested change
* @throws {ParseJsonError} when the json string is not valid json
* @example
* ```ts
* import { runPromise } from 'effect/Effect'
* import { parseJson } from '@tevm/effect'
*
* const jsonEffect = parseJson('{ "compilerOptions": { "plugins": [{ "name": "@tevm/ts-plugin" }] } }')
* ````
* const json = await runPromise(jsonEffect)
* ```
* `@throws` {ParseJsonError} when running the returned Effect fails to parse
* `jsonStr` with the configured JSONC options
* `@example`
*
🤖 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/effect/src/parseJson.js` around lines 26 - 34, Update the JSDoc for
parseJson to describe failures as invalid JSONC according to the parser’s
supported format, including allowed comments and trailing commas, rather than
strictly invalid JSON. Clarify that ParseJsonError is raised when the returned
Effect is executed, while parseJson itself returns an Effect.

Comment on lines +38 to +44
* import { tap } from 'effect/Effect'
* import { resolveSync } from '@tevm/effect'
*
* resolveSync('react', { basedir: process.cwd() }).pipe(
* tap(console.log)
* )
* ````
* `
* ```

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate resolve.js =="
fd -a 'resolve\.js$' . || true

echo "== relevant file excerpt =="
if [ -f packages/effect/src/resolve.js ]; then
  nl -ba packages/effect/src/resolve.js | sed -n '1,110p'
fi

echo "== effect/run exports in local repository =="
rg -n "export const (runSync|runPromise|runFiber)|function (runSync|runPromise|runFiber)|const (runSync|runPromise|runFiber)" packages/effect src . --glob '!*node_modules*' --glob '!*dist*' || true

echo "== package/typescript config context =="
for f in packages/effect/package.json packages/effect/tsconfig.json packages/effect/biome.json; do
  if [ -f "$f" ]; then
    echo "-- $f --"
    cat "$f"
  fi
done

Repository: evmts/tevm

Length of output: 290


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== packages/effect/package.json =="
python3 - <<'PY'
from pathlib import Path
p=Path('packages/effect/package.json')
print(p.read_text())
PY

echo "== resolve.js first 120 lines =="
python3 - <<'PY'
from pathlib import Path
p=Path('packages/effect/src/resolve.js')
for i, line in enumerate(p.read_text().splitlines(), 1):
    if i <= 120:
        print(f"{i:5d}: {line}")
PY

echo "== docs/API references in repo (Effect runtime helpers) =="
python3 - <<'PY'
from pathlib import Path
terms=['runSync','runPromise','runFiber','Effect.runSync','Effect.runPromise']
for term in terms:
    print(f"-- {term} --")
    for path in Path('.').rglob('*'):
        if path.is_file() and 'node_modules' not in path.parts:
            try:
                txt=path.read_text(encoding='utf-8', errors='ignore')
            except Exception:
                continue
            if term in txt:
                print(path)
PY

Repository: evmts/tevm

Length of output: 9512


🌐 Web query:

Effect.ts Effect runSync runPromise import from effect documentation

💡 Result:

In Effect.ts, runSync and runPromise are functions provided by the Runtime module, which is used to execute Effect programs [1][2]. Because they are effectful, they should only be used at the absolute edges of your program (e.g., at the entry point of your application) [1][2]. You can access these functions by importing the Runtime module from the effect package [2]. Usage Examples: import { Runtime, Effect } from "effect" // To run as a Promise Runtime.runPromise(Runtime.defaultRuntime)(Effect.succeed(42)).then(console.log) // To run synchronously const result = Runtime.runSync(Runtime.defaultRuntime)(Effect.succeed(42)) Key Considerations: 1. Runtime: Both functions require a Runtime instance, such as Runtime.defaultRuntime, to execute the effect [1][2]. 2. runPromise: Returns a Promise that resolves with the effect's value or rejects if the effect fails [1][2]. 3. runSync: Executes the effect synchronously and returns the result [1][2]. It will throw an exception if the effect encounters an error or an asynchronous boundary [1][2]. 4. Production Advice: For production applications, especially those requiring proper signal handling and graceful shutdown, it is recommended to use platform-specific entry points like NodeRuntime.runMain (from @effect/platform-node) instead of manual runPromise or runSync calls [3][4].

Citations:


Run both resolve Effects in the examples.

Both snippets compose an Effect without executing it, so tap will never run as written.

  • resolveSync example: wrap resolveSync(...).pipe(tap(console.log)) with an Effect runtime execution.
  • resolveAsync example: wrap resolveAsync(...).pipe(tap(console.log)) with async runtime execution and await.
📍 Affects 1 file
  • packages/effect/src/resolve.js#L38-L44 (this comment)
  • packages/effect/src/resolve.js#L55-L65
🤖 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/effect/src/resolve.js` around lines 38 - 44, Execute both Effect
examples in packages/effect/src/resolve.js: update the resolveSync example
around resolveSync and tap to run through the Effect runtime, and update the
resolveAsync example around resolveAsync and tap to use async runtime execution
with await. Ensure both examples actually execute so console.log is invoked.

Source: Coding guidelines

Comment on lines +7 to +25
* @param {{request: import('viem').EIP1193RequestFn}} client - An EIP-1193 compatible client such as a viem client
* @returns {import("./JsonRpcClient.js").JsonRpcClient} A JSON-RPC client whose `request` method resolves with the full JSON-RPC response (including `result` or `error`)
* @example
* ```typescript
* const url = 'https://mainnet.optimism.io'
* const params = {
* import { createJsonRpcFetcher } from '@tevm/jsonrpc'
* import { createPublicClient, http } from 'viem'
* import { optimism } from 'viem/chains'
*
* const client = createPublicClient({
* chain: optimism,
* transport: http('https://mainnet.optimism.io'),
* })
* const fetcher = createJsonRpcFetcher(client)
*
* const { result: block } = await fetcher.request({
* method: 'eth_getBlockByNumber',
* params: ['latest', false],
* }
* const {result: block} = await fetchJsonRpc(url, params)
* })
* console.log(block.number)

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Locate files:"
fd -a 'createJsonRpcFetcher\.js|JsonRpcClient\.js' . || true

echo
echo "Relevant file outline/content:"
if [ -f packages/jsonrpc/src/createJsonRpcFetcher.js ]; then
  wc -l packages/jsonrpc/src/createJsonRpcFetcher.js
  cat -n packages/jsonrpc/src/createJsonRpcFetcher.js
fi

echo
if [ -f packages/jsonrpc/src/JsonRpcClient.js ]; then
  wc -l packages/jsonrpc/src/JsonRpcClient.js
  cat -n packages/jsonrpc/src/JsonRpcClient.js
fi

echo
echo "Search related implementations/usages:"
rg -n "createJsonRpcFetcher|JsonRpcClient|request\\(|errorResponse|error:" packages/jsonrpc/src -S || true

Repository: evmts/tevm

Length of output: 5683


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "JsonRpcClient.ts:"
cat -n packages/jsonrpc/src/JsonRpcClient.ts

echo
echo "JsonRpcResponse.ts:"
cat -n packages/jsonrpc/src/JsonRpcResponse.ts

echo
echo "createJsonRpcFetcher spec:"
cat -n packages/jsonrpc/src/createJsonRpcFetcher.spec.ts

echo
echo "Behavioural probe for documented response shapes:"
node - <<'JS'
function createProbeFetcher(mockRequest) {
  return {
    request: async (request) => {
      try {
        const result = await mockRequest(request)
        return {
          jsonrpc: '2.0',
          method: request.method,
          result,
          ...(request.id !== undefined ? { id: request.id } : {}),
        }
      } catch (e) {
        const message =
          typeof e === 'object' && e !== null && 'message' in e && typeof e.message === 'string'
            ? e.message
            : typeof e === 'string'
              ? e
              : 'Unknown error in jsonrpc request'
        return {
          jsonrpc: '2.0',
          method: request.method,
          error: { code: typeof e?.code === 'number' ? e.code : -32000, message },
          ...(request.id !== undefined ? { id: request.id } : {}),
        }
      }
    },
  }
}
const requestHeaders = {
  blockNumber: { response: null },
  errorResponse: { throws: new Error('RPC error') },
  successResponse: { response: { number: '0x1' } },
}
for (const [name, request] of Object.entries(requestHeaders)) {
  const fetcher = createProbeFetcher(async () => {
    if (request.throws) throw request.throws
    return request.response
  })
  fetcher.request({ method: 'eth_getBlockByNumber', params: ['latest', false] })
    .then(async response => {
      console.log(`case=${name}`)
      console.log(JSON.stringify({ response, errorPresent: 'error' in response, resultPresent: 'result' in response }, null, 2))
      try {
        const { result: block } = response
        console.log(`dereferenced=${block.value}`)
      } catch (e) {
        console.log(`dereference_threw=${e.name}: ${e.message}`)
      }
    })
}
JS

Repository: evmts/tevm

Length of output: 7649


Handle error and null responses before reading the block.

request can return an error response for RPC failures, and eth_getBlockByNumber can also return result: null, so const { result: block } = ...; console.log(block.number) can throw in documented response cases. Re-read the response, handle error, and guard against a missing block before printing block.number.

🤖 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/jsonrpc/src/createJsonRpcFetcher.js` around lines 7 - 25, Update the
createJsonRpcFetcher usage example so it checks the full response from
fetcher.request for an RPC error before accessing result, then guards against a
null or missing block before printing block.number. Preserve the existing
eth_getBlockByNumber request and successful-response behavior while
demonstrating safe handling of both failure cases.

Source: Coding guidelines

Comment on lines +15 to +38
* const persister = createSyncStoragePersister({
* storage: window.localStorage,
* key: 'TEVM_CACHE',
* throttleTime: 1000,
* })
*
* // Persist state (throttled)
* persister.persistTevmState({
* '0x420': {
* balance: '0x69',
* codeHash: '0xdeadbeef',
* nonce: '0x0',
* storageRoot: '0xdeadbeef',
* storage: {
* '0x420420': '0x42069',
* },
* },
* })
*
* // Restore the persisted state later
* const restoredState = persister.restoreState()
*
* // Remove the persisted state
* persister.removePersistedState()

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Wait for throttled persistence before restoring.

With throttleTime: 1000, restoreState() runs before persistence is guaranteed to complete, and removePersistedState() can cancel the pending save. Add a delay before restoring, or use throttleTime: 0 for a deterministic example.

As per coding guidelines, “We always make the examples working examples.”

Proposed fix
 * })
 *
 * // Persist state (throttled)
 * persister.persistTevmState({
@@
 *   },
 * })
 *
+ * await new Promise((resolve) => setTimeout(resolve, 1100))
+ *
 * // Restore the persisted state later
 * const restoredState = persister.restoreState()
📝 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.

Suggested change
* const persister = createSyncStoragePersister({
* storage: window.localStorage,
* key: 'TEVM_CACHE',
* throttleTime: 1000,
* })
*
* // Persist state (throttled)
* persister.persistTevmState({
* '0x420': {
* balance: '0x69',
* codeHash: '0xdeadbeef',
* nonce: '0x0',
* storageRoot: '0xdeadbeef',
* storage: {
* '0x420420': '0x42069',
* },
* },
* })
*
* // Restore the persisted state later
* const restoredState = persister.restoreState()
*
* // Remove the persisted state
* persister.removePersistedState()
* const persister = createSyncStoragePersister({
* storage: window.localStorage,
* key: 'TEVM_CACHE',
* throttleTime: 1000,
* })
*
* // Persist state (throttled)
* persister.persistTevmState({
* '0x420': {
* balance: '0x69',
* codeHash: '0xdeadbeef',
* nonce: '0x0',
* storageRoot: '0xdeadbeef',
* storage: {
* '0x420420': '0x42069',
* },
* },
* })
*
* await new Promise((resolve) => setTimeout(resolve, 1100))
*
* // Restore the persisted state later
* const restoredState = persister.restoreState()
*
* // Remove the persisted state
* persister.removePersistedState()
🤖 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/sync-storage-persister/src/createSyncStoragePersister.js` around
lines 15 - 38, Update the example around createSyncStoragePersister so
restoreState() runs only after the throttled persistTevmState() operation has
completed, and ensure removePersistedState() does not cancel the pending save.
Use an awaited delay or set throttleTime to 0, while keeping the example
executable and deterministic.

Source: Coding guidelines

Comment on lines +7 to +14
* @example
* ```typescript
* import { noopPersister } from '@tevm/sync-storage-persister'
*
* // Safe to call every method; nothing is persisted
* noopPersister.persistTevmState({})
* const state = noopPersister.restoreState() // undefined
* noopPersister.removePersistedState()

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use a type-valid state in the TypeScript example.

persistTevmState accepts SerializableTevmState | undefined; {} is not a valid state and makes the example fail type checking. Pass undefined or provide a complete state object.

As per coding guidelines, “We always make the examples working examples.”

Proposed fix
-noopPersister.persistTevmState({})
+noopPersister.persistTevmState(undefined)
📝 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.

Suggested change
* @example
* ```typescript
* import { noopPersister } from '@tevm/sync-storage-persister'
*
* // Safe to call every method; nothing is persisted
* noopPersister.persistTevmState({})
* const state = noopPersister.restoreState() // undefined
* noopPersister.removePersistedState()
* `@example`
*
🤖 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/sync-storage-persister/src/noopPersister.js` around lines 7 - 14,
Update the noopPersister JSDoc example to pass undefined or a complete
SerializableTevmState to persistTevmState instead of {}. Keep the example’s
demonstrated no-op behavior and restoreState usage unchanged.

Source: Coding guidelines

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.

1 participant