-
Notifications
You must be signed in to change notification settings - Fork 0
Fine-grained exports from @typeberry/lib #852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
tomusdrw
commented
Dec 31, 2025
- More convenient library imports
- Add hash and pvm examples.
- Refine basic examples
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR converts the library to subpath-based exports by adding ~30 re-export shims under bin/lib/exports/* and updating bin/lib/index.ts to reference those shims (including several deprecated type-only re-exports). It adds example tests (bin/lib/examples/), updates README and typedoc/tsconfig, introduces bin/lib/package.json exports and scripts, adds an extract-examples script, and changes build tooling (packages/misc/builder/ and packages/misc/builder/rollup.config.js). A .coderabbit.yaml path_instructions rule requiring bin/lib/exports/.ts for new Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-06-18T20:35:13.536ZApplied to files:
📚 Learning: 2025-06-18T20:35:13.536ZApplied to files:
📚 Learning: 2025-09-15T14:37:04.214ZApplied to files:
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/misc/builder/tsconfig.json (1)
3-3: Consider a more targeted include pattern.The pattern
../../../**/*.tsis extremely broad and will include all TypeScript files from the repository root, potentially including test files, examples, and other non-build sources. This may impact build performance and IDE responsiveness.🔎 Suggested alternative
If the goal is to include the new export barrels under
bin/lib/exports, consider a more specific pattern:- "include": ["../../../**/*.ts"], + "include": [ + "../../../bin/lib/exports/**/*.ts", + "../../../packages/**/*.ts" + ],Or if you need to include specific directories, list them explicitly rather than using a catch-all pattern.
bin/lib/scripts/extract-examples.mjs (1)
24-26: Consider simplifying the regex usage.Line 25 creates a new
RegExpfromdynamicImportRegex, but sincematchAll()accepts the regex directly, this is unnecessary.🔎 Suggested simplification
- let match; - const regex = new RegExp(dynamicImportRegex); - for (match of code.matchAll(regex)) { + for (const match of code.matchAll(dynamicImportRegex)) {bin/lib/examples/pvm-usage.test.ts (1)
5-5: Consider importingStatusvia the subpath for consistency.The example imports use the new subpath pattern (
@typeberry/lib/pvm-interface), butStatusis imported directly from@typeberry/pvm-interface. For consistency with the example code patterns being demonstrated, consider:-import { Status } from "@typeberry/pvm-interface"; +// Status could be imported dynamically within the test like other modules, +// or keep as-is since it's test infrastructure, not example code.This is minor since Status is used outside the example markers (
<!-- example:... -->), so it doesn't affect the extracted documentation.bin/lib/examples/basic-usage.test.ts (1)
3-3: Import inconsistency within the example code.
BytesBlobis imported at line 3 from@typeberry/bytesdirectly, but it's used within the example markers (line 21). This doesn't align with the subpath import pattern being demonstrated. Consider importing it via the subpath within the example:-import { BytesBlob } from "@typeberry/bytes";And within the example block (around line 12):
const { BytesBlob } = await import("@typeberry/lib/bytes");Alternatively, move the import inside the example block to match the dynamic import pattern used for other modules.
Also applies to: 19-22
packages/misc/builder/setup.cjs (1)
133-157: Silent skip when export pattern doesn't match.When the source file doesn't match the expected
export * from "..."pattern, the entry is silently skipped (line 140-142). This could lead to missing exports in the final build without any warning. Consider adding a console warning:if (!match) { - // Skip files that don't match the expected export pattern - continue; + console.warn(`Warning: Skipping ${subpath.name} - source file doesn't match expected export pattern`); + continue; }bin/lib/index.ts (1)
29-30: Unusual alias:typeberryfor importer module.The
typeberrynamespace alias for the importer module is non-obvious. If this is for backwards compatibility, consider adding a comment explaining the historical reason. Otherwise,importerwould be more intuitive.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
.coderabbit.yamlbin/lib/README.mdbin/lib/examples/basic-usage.test.tsbin/lib/examples/bytes-usage.test.tsbin/lib/examples/codec-usage.test.tsbin/lib/examples/hash-usage.test.tsbin/lib/examples/pvm-usage.test.tsbin/lib/exports/block-json.tsbin/lib/exports/block.tsbin/lib/exports/bytes.tsbin/lib/exports/codec.tsbin/lib/exports/collections.tsbin/lib/exports/config-node.tsbin/lib/exports/config.tsbin/lib/exports/crypto.tsbin/lib/exports/database.tsbin/lib/exports/erasure-coding.tsbin/lib/exports/fuzz-proto.tsbin/lib/exports/hash.tsbin/lib/exports/importer.tsbin/lib/exports/jam-host-calls.tsbin/lib/exports/json-parser.tsbin/lib/exports/logger.tsbin/lib/exports/mmr.tsbin/lib/exports/numbers.tsbin/lib/exports/ordering.tsbin/lib/exports/pvm-host-calls.tsbin/lib/exports/pvm-interface.tsbin/lib/exports/pvm-interpreter.tsbin/lib/exports/shuffling.tsbin/lib/exports/state-json.tsbin/lib/exports/state-merkleization.tsbin/lib/exports/state-vectors.tsbin/lib/exports/state.tsbin/lib/exports/transition.tsbin/lib/exports/trie.tsbin/lib/exports/utils.tsbin/lib/exports/workers-api.tsbin/lib/index.tsbin/lib/package.jsonbin/lib/scripts/extract-examples.mjsbin/pvm/index.tspackage.jsonpackages/core/ordering/index.tspackages/core/pvm-interpreter/bin.tspackages/core/pvm-interpreter/interpreter.tspackages/misc/builder/rollup.config.jspackages/misc/builder/setup.cjspackages/misc/builder/tsconfig.jsontsconfig.jsontypedoc.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: rules from./CODESTYLE.mdshould be adhered to.
**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in*.test.tsfiles. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.
**/*.ts:asconversions must not be used. Suggest usingtryAsconversion methods.
**/*.ts: Classes withstatic Codecfield must have private constructor and staticcreatemethod.
**/*.ts: Casting abigint(orU64) usingNumber(x)must have an explanation comment why
it is safe.
**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.
Files:
bin/lib/exports/codec.tsbin/lib/exports/fuzz-proto.tsbin/lib/exports/pvm-interpreter.tsbin/lib/exports/mmr.tsbin/lib/exports/utils.tsbin/lib/exports/crypto.tsbin/lib/exports/json-parser.tsbin/pvm/index.tsbin/lib/exports/hash.tsbin/lib/exports/erasure-coding.tsbin/lib/exports/bytes.tsbin/lib/exports/collections.tsbin/lib/exports/state.tsbin/lib/exports/state-vectors.tsbin/lib/exports/ordering.tsbin/lib/exports/config-node.tsbin/lib/examples/codec-usage.test.tsbin/lib/exports/config.tsbin/lib/exports/importer.tsbin/lib/exports/transition.tsbin/lib/exports/trie.tsbin/lib/exports/pvm-host-calls.tsbin/lib/examples/basic-usage.test.tsbin/lib/examples/pvm-usage.test.tsbin/lib/examples/hash-usage.test.tspackages/core/ordering/index.tspackages/core/pvm-interpreter/interpreter.tsbin/lib/exports/state-json.tsbin/lib/exports/shuffling.tsbin/lib/exports/logger.tspackages/core/pvm-interpreter/bin.tsbin/lib/exports/block-json.tsbin/lib/exports/block.tsbin/lib/examples/bytes-usage.test.tsbin/lib/exports/workers-api.tsbin/lib/exports/database.tsbin/lib/exports/numbers.tsbin/lib/exports/jam-host-calls.tsbin/lib/exports/state-merkleization.tsbin/lib/exports/pvm-interface.tsbin/lib/index.ts
packages/core/**/*.ts
⚙️ CodeRabbit configuration file
packages/core/**/*.ts: Core packages must not import any JAM-related packages
(i.e. packages defined underpackages/jam/**)
Files:
packages/core/ordering/index.tspackages/core/pvm-interpreter/interpreter.tspackages/core/pvm-interpreter/bin.ts
bin/lib/index.ts
⚙️ CodeRabbit configuration file
bin/lib/index.ts: Every newexport * as <name>statement must have a corresponding entry point file
atbin/lib/exports/<name>.tsthat re-exports the package (e.g.,export * from "@typeberry/<package>").
The builder will automatically detect these files and generate the appropriate exports.
Files:
bin/lib/index.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 625
File: benchmarks/package.json:8-15
Timestamp: 2025-09-15T14:37:04.214Z
Learning: In the FluffyLabs/typeberry monorepo, the workspace:* protocol does not work for internal typeberry dependencies. The working approach is to use "*" as the version specifier for internal monorepo packages instead of "workspace:*".
📚 Learning: 2025-11-03T10:12:27.580Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 757
File: packages/jam/state-vectors/index.ts:18-36
Timestamp: 2025-11-03T10:12:27.580Z
Learning: In TypeScript files, the coding guideline requiring classes with `static Codec` field to have a private constructor and static `create` method applies specifically to classes using `codec.Class`. When using `codec.object`, this pattern is not required because the code only cares about the shape, not the instance.
Applied to files:
bin/lib/exports/codec.tsbin/lib/examples/codec-usage.test.ts
📚 Learning: 2025-09-15T14:37:04.214Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 625
File: benchmarks/package.json:8-15
Timestamp: 2025-09-15T14:37:04.214Z
Learning: In the FluffyLabs/typeberry monorepo, the workspace:* protocol does not work for internal typeberry dependencies. The working approach is to use "*" as the version specifier for internal monorepo packages instead of "workspace:*".
Applied to files:
bin/lib/package.jsonbin/lib/README.mdbin/lib/index.ts
📚 Learning: 2025-05-26T21:31:58.688Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 399
File: .github/workflows/vectors-jamduna.yml:78-78
Timestamp: 2025-05-26T21:31:58.688Z
Learning: In the typeberry project, the npm start script in the test-runner workspace is designed to accept test suite names as arguments (e.g., "jamduna", "w3f"). The command `npm start -w typeberry/test-runner jamduna` is the correct way to run the jamduna test suite, not `npm run jamduna`. This is an architectural design choice where the start script acts as a unified entry point for different test suites.
Applied to files:
bin/lib/package.jsonpackage.json
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `typeberry/utils` package has browser compatibility issues due to Node.js-specific code like `measure` function using `process.hrtime()` and `testUtils` importing `node:assert`, causing white screens in browser environments.
Applied to files:
bin/lib/exports/utils.tsbin/lib/exports/config-node.tsbin/lib/examples/basic-usage.test.tsbin/lib/README.mdbin/lib/examples/bytes-usage.test.tsbin/lib/index.ts
📚 Learning: 2025-04-24T19:42:18.083Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 351
File: bin/jam/genesis.ts:7-11
Timestamp: 2025-04-24T19:42:18.083Z
Learning: Error handling for file operations and JSON parsing in the typeberry project is done at a higher level in the call stack rather than within individual utility functions.
Applied to files:
bin/lib/exports/utils.tsbin/lib/exports/json-parser.ts
📚 Learning: 2025-04-24T19:48:31.051Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 351
File: packages/jam/state-merkleization/index.ts:19-37
Timestamp: 2025-04-24T19:48:31.051Z
Learning: In the typeberry codebase, `Bytes` inherits from `BytesBlob`, so a `Bytes` instance can be directly used where a `BytesBlob` is expected without needing conversion.
Applied to files:
bin/lib/exports/bytes.tsbin/lib/examples/bytes-usage.test.ts
📚 Learning: 2025-06-10T12:06:32.535Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/jam/database-lmdb/states.test.ts:158-160
Timestamp: 2025-06-10T12:06:32.535Z
Learning: In test code, tomusdrw prefers pragmatic solutions over strict type safety when listing all entries would be cumbersome. Using `Object.assign({}, state)` for state updates in tests is acceptable even if it compromises type safety, prioritizing test maintainability and readability.
Applied to files:
bin/lib/exports/state-vectors.ts
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `measure` function in `typeberry/utils/debug.ts` attempts environment detection by checking `process === undefined` but still causes bundling issues because bundlers see the `process.hrtime` reference in the Node.js branch.
Applied to files:
bin/lib/exports/config-node.ts
📚 Learning: 2025-06-10T12:11:27.374Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 419
File: packages/core/trie/nodesDb.ts:26-33
Timestamp: 2025-06-10T12:11:27.374Z
Learning: In the trie implementation (`packages/core/trie/nodesDb.ts`), the `leaves()` method intentionally yields both `NodeType.Leaf` and `NodeType.EmbedLeaf` nodes by filtering out only `NodeType.Branch` nodes. Both leaf types are valid leaf nodes that should be included in leaf iteration.
Applied to files:
bin/lib/exports/trie.ts
📚 Learning: 2025-05-16T16:23:08.685Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 390
File: packages/jam/jam-host-calls/fetch.ts:220-226
Timestamp: 2025-05-16T16:23:08.685Z
Learning: In the typeberry codebase, host call (HC) implementations define a gas cost but don't need to explicitly consume gas in their execute() methods. Gas accounting is handled outside of the host call implementation.
Applied to files:
bin/lib/exports/pvm-host-calls.tsbin/lib/exports/jam-host-calls.ts
🧬 Code graph analysis (5)
bin/lib/examples/codec-usage.test.ts (4)
packages/core/codec/index.ts (1)
codec(12-12)packages/core/bytes/bytes.ts (1)
Bytes(177-243)packages/core/codec/encoder.ts (1)
Encoder(73-480)packages/core/codec/decoder.ts (1)
Decoder(12-379)
bin/lib/examples/pvm-usage.test.ts (3)
packages/core/bytes/bytes.ts (1)
BytesBlob(18-172)packages/core/pvm-interpreter/interpreter.ts (1)
Interpreter(61-325)packages/core/pvm-interface/gas.ts (1)
tryAsGas(18-19)
bin/lib/examples/hash-usage.test.ts (1)
packages/core/hash/blake2b.ts (1)
Blake2b(8-44)
packages/core/pvm-interpreter/interpreter.ts (1)
packages/core/pvm-interpreter/assemblify.ts (1)
assemblify(209-221)
bin/lib/examples/bytes-usage.test.ts (1)
packages/core/bytes/bytes.ts (2)
BytesBlob(18-172)Bytes(177-243)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup test vectors cache
- GitHub Check: benchmarks (22.x)
🔇 Additional comments (59)
package.json (1)
73-73: LGTM!The docs script now runs workspace-level documentation generation before the root-level TypeDoc build, which is a good pattern for ensuring all sub-package documentation is generated first.
.coderabbit.yaml (1)
30-34: LGTM!The new path instruction clearly documents the expected pattern for export statements in
bin/lib/index.ts, ensuring that each namespace export has a corresponding entry point file. This will help maintain consistency as the export surface evolves.bin/lib/exports/pvm-interface.ts (1)
1-9: LGTM!This export barrel follows the established pattern for the new subpath exports model. The JSDoc comment clearly describes the module's purpose, and the re-export from
@typeberry/pvm-interfaceis straightforward.tsconfig.json (1)
3-3: This is a standard monorepo configuration that requires no special verification.Setting
rootDirto"."in the roottsconfig.jsonis the correct approach for a monorepo with workspaces. Combined withnoEmit: true, it ensures TypeScript correctly resolves module paths across all workspace packages without generating output files. IDE type checking and relative imports will work as expected across thebin/andpackages/workspaces.bin/lib/exports/pvm-host-calls.ts (1)
1-9: LGTM! Clean re-export barrel.The structure and documentation are correct. This follows the established pattern for the new subpath exports architecture.
bin/lib/exports/numbers.ts (1)
1-9: LGTM! Clean re-export barrel.The structure and documentation are correct. This follows the established pattern for the new subpath exports architecture.
bin/lib/exports/state.ts (1)
1-9: LGTM! Clean re-export barrel.The structure and documentation are correct. This follows the established pattern for the new subpath exports architecture.
bin/lib/exports/mmr.ts (1)
1-9: LGTM! Clean re-export barrel.The structure and documentation are correct. This follows the established pattern for the new subpath exports architecture.
bin/lib/exports/utils.ts (1)
1-9: LGTM! Note browser compatibility concerns.The structure and documentation are correct. However, be aware that
@typeberry/utilshas known browser compatibility issues due to Node.js-specific code (process.hrtime()andnode:assertimports). Ensure consumers are aware of this limitation when using utils in browser environments.Based on learnings, the utils package contains browser-incompatible code.
bin/lib/exports/collections.ts (1)
1-9: LGTM!The re-export barrel is correctly structured with appropriate module documentation.
bin/lib/exports/config.ts (1)
1-9: LGTM!The configuration re-export barrel follows the correct pattern with clear module documentation.
bin/lib/exports/crypto.ts (1)
1-9: LGTM!The cryptographic primitives re-export is properly structured with comprehensive module documentation.
bin/lib/exports/state-merkleization.ts (1)
1-9: LGTM!The state merkleization re-export barrel is correctly implemented with clear documentation.
bin/lib/scripts/extract-examples.mjs (5)
1-11: LGTM!The setup and constants are correctly configured for an ESM script.
76-117: LGTM!The example extraction logic correctly handles indentation normalization and integrates with the dynamic import conversion. The regex matching and string processing are appropriate for the use case.
122-140: LGTM!The helper functions correctly aggregate examples from all test files. The use of
Object.assignwill silently overwrite duplicate example names if they exist across files, which appears to be acceptable behavior for this tooling.
145-167: LGTM!The README update logic correctly handles placeholder replacement with proper regex escaping. The conditional check at line 161 ensures placeholders are only replaced if they exist.
169-182: LGTM!The main execution flow is clear and includes appropriate logging for visibility during the documentation build process.
bin/pvm/index.ts (1)
18-19: LGTM! API migration is correct.The change from
printProgram()todumpProgram()aligns with the updated PVM interpreter API. The addition ofconsole.table()provides a nice tabular view of the instructions.bin/lib/exports/fuzz-proto.ts (1)
1-9: LGTM! Clean re-export barrel.The module documentation is clear, and the re-export pattern is consistent with the PR's goal of creating fine-grained local export surfaces.
bin/lib/exports/bytes.ts (1)
1-9: LGTM! Clean re-export barrel.The module provides a clean local export surface for byte utilities with appropriate documentation.
bin/lib/exports/trie.ts (1)
1-9: LGTM! Clean re-export barrel.The module documentation clearly describes trie data structures and Merkle proof generation. Implementation follows the established pattern for local export surfaces.
bin/lib/exports/codec.ts (1)
1-9: LGTM! Clean re-export barrel.The module documentation is clear and the re-export pattern is appropriate for establishing a clean public API surface.
bin/lib/exports/hash.ts (1)
1-9: LGTM! Well-documented export barrel.The documentation clearly describes the hashing capabilities and the re-export is straightforward.
bin/lib/exports/pvm-interpreter.ts (1)
1-9: LGTM! Consistent export barrel.The PVM interpreter documentation is clear and the re-export follows the established pattern.
bin/lib/exports/json-parser.ts (1)
1-9: LGTM! Clean and well-documented barrel.The JSON parsing documentation is appropriate and the re-export maintains the established pattern across all export barrels.
typedoc.json (1)
7-7: The TypeDoc glob pattern is correctly configured and will process all export barrels.The glob pattern
./bin/lib/exports/*is valid TypeDoc syntax (minimatch-style) and successfully matches all 31 export barrel files in the directory. Each barrel file properly re-exports from @typeberry modules and includes JSDoc documentation. TypeDoc will discover and generate documentation for all intended modules with this configuration.bin/lib/exports/database.ts (1)
1-9: LGTM!The re-export barrel is well-documented and correctly structured. This aligns with the PR's objective of providing fine-grained exports from
@typeberry/lib.packages/core/ordering/index.ts (1)
2-9: LGTM!Exporting
OrderingValueis a reasonable API expansion since theOrderingclass already exposes these values through its static properties. This allows consumers to work with ordering values more directly.bin/lib/exports/state-json.ts (1)
1-9: LGTM!The re-export barrel is well-documented and follows the consistent pattern established for the new subpath exports.
bin/lib/exports/state-vectors.ts (1)
1-9: LGTM!The re-export barrel is properly structured with clear documentation. Consistent with the new export pattern.
bin/lib/examples/codec-usage.test.ts (1)
1-25: LGTM!The codec example demonstrates proper usage of the new subpath exports:
- Correct dynamic imports from
@typeberry/lib/codecand@typeberry/lib/bytes- Proper encoding/decoding workflow with fixed-size bytes
- Sound test logic with appropriate assertions
- Example markers for documentation extraction
bin/lib/exports/transition.ts (1)
1-9: LGTM! Clean re-export barrel with clear documentation.This file follows the established pattern for the subpath exports refactoring, providing a convenient entry point for transition-related functionality with appropriate JSDoc documentation.
packages/core/pvm-interpreter/bin.ts (2)
4-7: LGTM! Program data update.The updated byte array appears to be part of the example/test program for the interpreter.
11-12: LGTM! Appropriate API migration and console usage.The change from
printProgram()todumpProgram()withconsole.table()aligns with the broader API evolution to separate data retrieval from output formatting. The biome-ignore comment appropriately justifies console usage in this executable bin file.bin/lib/exports/workers-api.ts (1)
1-9: LGTM! Consistent re-export pattern.This barrel export follows the same clean pattern established across the PR, providing convenient access to Workers API functionality with clear documentation.
bin/lib/exports/config-node.ts (1)
1-9: LGTM! Appropriately scoped Node-specific exports.This barrel correctly isolates Node.js-specific configuration utilities, with clear documentation indicating its environment constraints. The explicit naming and documentation help prevent unintended usage in browser environments.
bin/lib/exports/erasure-coding.ts (1)
1-9: LGTM! Clear re-export with informative documentation.This barrel export maintains consistency with the subpath exports pattern and provides helpful context about the erasure coding functionality's role in the JAM data availability system.
bin/lib/exports/logger.ts (1)
1-9: LGTM! Clean re-export barrel.The module documentation is clear and the re-export follows the established pattern for the new subpath exports structure.
bin/lib/exports/ordering.ts (1)
1-9: LGTM! Consistent barrel export.The re-export follows the established pattern and provides clear module documentation.
bin/lib/exports/block.ts (1)
1-9: LGTM! Well-documented barrel export.The module documentation clearly describes the JAM protocol block structures, and the re-export is clean and consistent with the refactoring pattern.
bin/lib/exports/shuffling.ts (1)
1-9: LGTM! Clean and well-documented.The module provides clear context about the shuffling utilities' role in the JAM protocol, and the re-export follows the established pattern.
packages/core/pvm-interpreter/interpreter.ts (1)
162-164: All call sites updated correctly; refactoring is complete.The rename from
printProgram()todumpProgram()has been fully applied. No references to the old method name remain, and all call sites have been updated to usedumpProgram()appropriately. The refactoring improves composability by returning data instead of handling output as a side effect—callers likebin/pvm/index.ts,packages/core/pvm-interpreter/bin.ts, and test files now explicitly invokeconsole.table()with the returned data.bin/lib/exports/block-json.ts (1)
1-9: LGTM! Clean re-export barrel.This file correctly establishes a local export surface for the block-json module with clear documentation. The pattern is consistent with the PR's objective of introducing fine-grained subpath exports.
bin/lib/exports/jam-host-calls.ts (1)
1-9: LGTM! Clean re-export barrel.This file correctly establishes a local export surface for JAM host calls with clear documentation. The pattern aligns with the broader refactoring to subpath exports.
bin/lib/examples/bytes-usage.test.ts (1)
1-33: LGTM! Well-structured example tests.The tests effectively demonstrate the bytes API usage:
- Parsing hex strings with
BytesBlob.parseBlob- Creating fixed-size bytes with
Bytes.fillThe dynamic imports align with the new subpath export strategy, and the example markers will enable documentation extraction.
bin/lib/examples/hash-usage.test.ts (1)
1-50: LGTM! Comprehensive hash usage examples.The tests thoroughly demonstrate Blake2b functionality:
- Hashing byte arrays with
hashBytes- Hashing strings directly with
hashString- Hashing multiple blobs with
hashBlobsAll tests correctly verify the 32-byte hash length and use the new subpath import strategy. The example markers support documentation generation.
bin/lib/package.json (2)
74-84: LGTM! Script additions support the new export workflow.The new scripts effectively support documentation generation and example extraction:
docs:examplesextracts examples from test filesprebuildensures examples are extracted before buildingtest:examplesprovides targeted testing for example codeThe addition of
"type": "module"correctly declares this as an ES module package.
6-40: No action needed — exports pointing to TypeScript source files is the correct pattern for this monorepo.The source
package.jsonexports reference.tsfiles by design. The build tool (setup.cjs+ rollup) reads these source exports, compiles them to.mjs,.cjs, and.d.tsfiles, and generates a newpackage.jsonin thedist/liboutput directory with exports pointing to the compiled artifacts. This is standard practice for TypeScript monorepos where the source tree references source files and the published package references build outputs.Likely an incorrect or invalid review comment.
bin/lib/examples/pvm-usage.test.ts (2)
8-32: LGTM!The test correctly demonstrates the PVM usage pattern with proper dynamic imports, program initialization, and execution. The example markers are well-placed for documentation extraction.
34-53: LGTM!The register access example is clear and demonstrates the BigInt return type correctly.
bin/lib/examples/basic-usage.test.ts (1)
26-39: LGTM!The numbers example correctly demonstrates the typed number utilities with proper assertions.
packages/misc/builder/setup.cjs (2)
122-131: LGTM!The main entry point configuration is correctly structured with ESM, CJS, and TypeScript declaration outputs.
41-60: LGTM!The subpath exports extraction logic correctly handles both string and object export configurations, properly normalizing paths.
packages/misc/builder/rollup.config.js (3)
10-19: LGTM!The external dependency logic correctly:
- Externalizes specific dependencies (
lmdb,@matrixai/quic, Node builtins)- Externalizes all
@typeberry/*packages except@typeberry/native(which should be bundled)- Catches all
node:*imports
76-91: LGTM!The dummy config fallback handles the edge case where all subpaths are generated directly by
setup.cjs, satisfying Rollup's requirement for at least one configuration. The.noop*pattern is correctly ignored in.npmignore.
40-74: LGTM!The config generation correctly produces separate CJS and ESM builds per entry, with conditional DTS generation. The
respectExternal: trueoption ensures external package types aren't inlined.bin/lib/index.ts (1)
1-66: Export structure adheres to coding guidelines.All
export * as <name>statements have corresponding entry point files inbin/lib/exports/. The namespace naming convention (underscores for exports, kebab-case for file paths) is correct and maintains backwards compatibility. The deprecation markers are consistently applied across all exports.bin/lib/README.md (1)
283-305: No changes needed. The gas comparison logic is valid.JavaScript comparison operators work correctly with mixed bigint and number types, so
remainingGas < initialGaswill work whetherGasis aSmallGas(number) orBigGas(bigint). TheOpaquetype system preserves the underlying type's operators, and the assertion correctly validates that gas was consumed during execution.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
bin/lib/README.md (4)
86-104: Missingassertimport (duplicate issue).The example uses
assert.equal()andassert.throws()without importingassert. Add the import at the top of the example:+import assert from "node:assert"; import { Decoder } from "@typeberry/lib/codec"; import { InMemoryState } from "@typeberry/lib/state"; import { BytesBlob } from "@typeberry/lib/bytes"; import { Block, tryAsServiceId } from "@typeberry/lib/block";
110-121: Missingassertimport (duplicate issue).The example uses
assert.ok()andassert.strictEqual()without importingassert. Add the import:+import assert from "node:assert"; import { tryAsU8, tryAsU32, isU8 } from "@typeberry/lib/numbers";
127-139: Missingassertimport in hash examples (duplicate issue).All three hash examples use
assert.strictEqual()without importingassert. Add the import to each example block:+import assert from "node:assert"; import { Blake2b } from "@typeberry/lib/hash";Also applies to: 145-155, 161-173
237-258: Missingassertimport in PVM examples (duplicate issue).All three PVM examples use
assertfunctions without importingassert. Add the import to each:+import assert from "node:assert"; import { Interpreter } from "@typeberry/lib/pvm-interpreter"; import { tryAsGas } from "@typeberry/lib/pvm-interface"; import { BytesBlob } from "@typeberry/lib/bytes";(The PVM-basic example already correctly imports
Statusfrom@typeberry/lib/pvm-interface, so only theassertimport needs to be added.)Also applies to: 265-281, 288-304
🧹 Nitpick comments (1)
bin/lib/examples/basic-usage.test.ts (1)
16-16: Consider explaining the entropy length magic number.The assertion checks that
entropy.lengthequals4, but this magic number lacks context. For a documentation example, consider adding a comment explaining why the entropy length is 4 for thetinyChainSpec, or reference a constant if available. This helps readers understand what they're seeing.💡 Example with explanatory comment
- assert.equal(state.entropy.length, 4); + // tinyChainSpec uses 4 bytes of entropy + assert.equal(state.entropy.length, 4);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bin/lib/README.mdbin/lib/examples/basic-usage.test.tsbin/lib/examples/pvm-usage.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- bin/lib/examples/pvm-usage.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts: rules from./CODESTYLE.mdshould be adhered to.
**/*.ts: Any function whose documentation mention it must not be used in production code,
can be safely used in*.test.tsfiles. Other usage should be carefuly reviewed
and the comment must explain why it's safe to use there.
**/*.ts:asconversions must not be used. Suggest usingtryAsconversion methods.
**/*.ts: Classes withstatic Codecfield must have private constructor and staticcreatemethod.
**/*.ts: Casting abigint(orU64) usingNumber(x)must have an explanation comment why
it is safe.
**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.
Files:
bin/lib/examples/basic-usage.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 625
File: benchmarks/package.json:8-15
Timestamp: 2025-09-15T14:37:04.214Z
Learning: In the FluffyLabs/typeberry monorepo, the workspace:* protocol does not work for internal typeberry dependencies. The working approach is to use "*" as the version specifier for internal monorepo packages instead of "workspace:*".
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `typeberry/utils` package has browser compatibility issues due to Node.js-specific code like `measure` function using `process.hrtime()` and `testUtils` importing `node:assert`, causing white screens in browser environments.
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `typeberry/utils` package has browser compatibility issues due to Node.js-specific code like `measure` function using `process.hrtime()` and `testUtils` importing `node:assert`, causing white screens in browser environments.
Applied to files:
bin/lib/examples/basic-usage.test.tsbin/lib/README.md
📚 Learning: 2025-06-18T20:35:13.536Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 442
File: packages/core/pvm-debugger-adapter/index.ts:22-40
Timestamp: 2025-06-18T20:35:13.536Z
Learning: The `measure` function in `typeberry/utils/debug.ts` attempts environment detection by checking `process === undefined` but still causes bundling issues because bundlers see the `process.hrtime` reference in the Node.js branch.
Applied to files:
bin/lib/README.md
📚 Learning: 2025-09-15T14:37:04.214Z
Learnt from: tomusdrw
Repo: FluffyLabs/typeberry PR: 625
File: benchmarks/package.json:8-15
Timestamp: 2025-09-15T14:37:04.214Z
Learning: In the FluffyLabs/typeberry monorepo, the workspace:* protocol does not work for internal typeberry dependencies. The working approach is to use "*" as the version specifier for internal monorepo packages instead of "workspace:*".
Applied to files:
bin/lib/README.md
🧬 Code graph analysis (1)
bin/lib/examples/basic-usage.test.ts (6)
packages/jam/state/in-memory-state.ts (1)
InMemoryState(195-603)packages/jam/block/common.ts (1)
tryAsServiceId(28-28)packages/core/codec/decoder.ts (1)
Decoder(12-379)packages/jam/block/block.ts (1)
Block(72-90)packages/core/bytes/bytes.ts (1)
BytesBlob(18-172)packages/core/numbers/index.ts (3)
tryAsU8(28-31)tryAsU32(45-48)isU8(33-33)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: benchmarks (22.x)
- GitHub Check: Setup test vectors cache
- GitHub Check: jam-e2e (22.x)
- GitHub Check: e2e (22.x)
- GitHub Check: build (22.x)
- GitHub Check: test (22.x)
🔇 Additional comments (3)
bin/lib/examples/basic-usage.test.ts (3)
1-2: LGTM: Standard Node.js test setup.The Node.js test framework imports are appropriate for this test file.
5-23: Excellent example demonstrating the new subpath imports.The test effectively demonstrates the new fine-grained import paths from
@typeberry/lib. All subpath imports are correctly configured:
@typeberry/lib/configexports the configuration module@typeberry/lib/codecexports the Decoder@typeberry/lib/stateexports InMemoryState@typeberry/lib/bytesexports BytesBlob@typeberry/lib/blockexports both Block and tryAsServiceIdThe HTML comment markers suggest this code is extracted for documentation, making the dynamic import syntax appropriate for showing users how to consume the library.
26-39: LGTM: Clear demonstration of numeric utilities.The test effectively demonstrates the usage of typed number utilities (
tryAsU8,tryAsU32,isU8) from the@typeberry/lib/numberssubpath export. The assertions correctly verify both type checking and value preservation.
Co-authored-by: Mateusz Sikora <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @bin/lib/scripts/extract-examples.mjs:
- Around line 27-39: The code destructures match into fullMatch, importsMatch
and modulePathMatch but then uses undefined modulePath and imports_; update all
references to use the destructured names: replace modulePath with
modulePathMatch when checking/setting importMap and when pushing into importMap,
and replace imports_ with importsMatch when splitting/cleaning import names and
when pushing into the imports array (i.e., imports.push should use imports:
importNames and modulePath: modulePathMatch). Ensure
importMap.get(modulePathMatch) is used consistently.
🧹 Nitpick comments (2)
bin/lib/scripts/extract-examples.mjs (2)
24-26: Simplify: Use the regex directly.Creating a new
RegExpfrom the existingdynamicImportRegexis redundant. UsedynamicImportRegexdirectly inmatchAll.🔎 Proposed fix
- let match; - const regex = new RegExp(dynamicImportRegex); - for (match of code.matchAll(regex)) { + for (const match of code.matchAll(dynamicImportRegex)) { const [fullMatch, importsMatch, modulePathMatch] = match;
45-45: Consider sorting imports for deterministic output.Sorting the unique imports lexically ensures consistent ordering across runs, making diffs cleaner.
🔎 Proposed fix
// Remove duplicates - const uniqueImports = [...new Set(importNames)]; + const uniqueImports = [...new Set(importNames)].sort(); staticImports.push(`import { ${uniqueImports.join(", ")} } from "${modulePath}";`);Based on past review comments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bin/lib/scripts/extract-examples.mjs
🧰 Additional context used
🪛 GitHub Actions: Build - Lint & Test
bin/lib/scripts/extract-examples.mjs
[error] 27-27: This variable importsMatch is unused.
[error] 27-27: This variable modulePathMatch is unused.
🪛 GitHub Actions: Build - Project
bin/lib/scripts/extract-examples.mjs
[error] 28-28: ReferenceError: modulePath is not defined in convertDynamicImportsToStatic
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Setup test vectors cache
- GitHub Check: benchmarks (22.x)
- GitHub Check: e2e (22.x)
- GitHub Check: jam-e2e (22.x)
View all
Benchmarks summary: 83/83 OK ✅ |