Fix create

[andrew-fleming]

The base create forced a fixed param shape and a generic return, so subclass overrides fail ts static-side checks. This PR makes create more permissive. Without doing a heavy refactor of typechecking, the evidence that this fix works is in the simulators in integration/. Without the fix, there's an ide error. With the fix, no error

Summary by CodeRabbit

• Bug Fixes
  • Improved simulator creation compatibility across different invocation patterns.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 756b3535-a7b4-4d77-8aee-ed7365c65f4d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

The simulator now uses separate TypeScript configurations for building and typechecking, updates its build script, generalizes the static factory signature, and adjusts witness override generic parameters.

Changes

Simulator TypeScript and factory updates

Layer / File(s)	Summary
Separate build and typecheck configurations packages/simulator/tsconfig.build.json, packages/simulator/tsconfig.json, packages/simulator/package.json	The package adds an emit-only build configuration, expands typechecking to source and test files, and points the build script to tsconfig.build.json.
Generalize factory and witness typing packages/simulator/src/factory/createSimulator.ts, packages/simulator/test/integration/Witness.test.ts	The static factory uses variadic arguments and returns Promise<Simulator>, while witness overrides use IWitnessWitnesses<unknown, WitnessPrivateState>.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• OpenZeppelin/compact-tools#73: Related witness generic refactoring.
• OpenZeppelin/compact-tools#121: Related createSimulator factory signature changes.

Suggested reviewers: 0xisk

Poem

A bunny builds with types so bright,
Separating checks from output night.
The factory takes a flexible leap,
While witnesses their shapes now keep.
Hop, compile, and declarations bloom!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title is too vague to convey the main change; it only says "Fix create" without indicating the simulator or TypeScript updates.	Rename the PR to describe the core change, such as updating the simulator create API and typecheck/build configs.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/simulator/src/factory/createSimulator.ts`:
- Around line 202-213: Update the static create method’s public
overload/signature to return the concrete constructor-derived subclass type
instead of Promise<Simulator>, while retaining a permissive implementation
signature for subclass overrides. Ensure await DerivedSimulator.create(...)
preserves DerivedSimulator-specific members, using the existing this constructor
type and create implementation.

🪄 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: 6b4ed645-9947-4a26-acb0-bef8e4951660

📥 Commits

Reviewing files that changed from the base of the PR and between bcce9b5 and 9d4281e.

📒 Files selected for processing (5)

• packages/simulator/package.json
• packages/simulator/src/factory/createSimulator.ts
• packages/simulator/test/integration/Witness.test.ts
• packages/simulator/tsconfig.build.json
• packages/simulator/tsconfig.json

[0xisk]

Reviewed ed07aec against origin/main (bcce9b5). 3 files, +15/−7.

Verified locally: reproduced the baseline TS2417 on all three sample simulators, confirmed this PR clears it, and ran the dry suite (10 files, 90/90 pass). I also tested two narrower fixes and both still fail, so ...args: unknown[] does look like the minimum that unblocks the override. Per-line notes are inline.

🔴 blocking — the fix has no regression guard

packages/simulator/tsconfig.json has include: ["src/**/*"], so yarn types never compiles test/integration/*Simulator.ts. yarn test is vitest, which strips types without checking them. The create signature can regress and CI stays green.

The same cause makes the Witness.test.ts hunk inert. It fixes a real arity bug (IWitnessWitnesses<L, P> takes two params), but nothing ever compiles that file, so it neither proves the fix nor guards it.

❔ question — why was 9d4281e reverted?

9d4281e had the split that answers the above: tsconfig.build.json for emit, tsconfig.json as noEmit + bundler + include: ["src/**/*", "test/**/*"]. ed07aec removed it. I ran that config verbatim against this branch and it was clean, apart from local @types/node resolution noise on my machine. If CI failed on it, that is worth recording in the PR body. If it was dropped to keep the diff small, it is the fix for the blocking item.

🔵 followup — README is stale on this exact API

packages/simulator/README.md still shows new MySimulator([...], {...}) (L37), BaseSimulatorOptions (L93, L262, L270), and a four-argument createSimulator<P, L, W, TArgs> that is missing the TContract slot (L59). Pre-existing rather than introduced here, but this PR is about create's contract and the README never documents create.

added by claude (dev3-code-review)

[andrew-fleming]

🔴 blocking — the fix has no regression guard
packages/simulator/tsconfig.json has include: ["src/**/*"], so yarn types never compiles test/integration/*Simulator.ts. yarn test is vitest, which strips types without checking them. The create signature can regress and CI stays green.

The same cause makes the Witness.test.ts hunk inert. It fixes a real arity bug (IWitnessWitnesses<L, P> takes two params), but nothing ever compiles that file, so it neither proves the fix nor guards it.

Added a regression test. I updated the description with the initial fix that the reason something like this wasn't included was because it would involve a non-trivial refactor on how types are checked. What this PR now proposes is an ugly fix

❔ question — why was 9d4281e reverted?
9d4281e had the split that answers the above: tsconfig.build.json for emit, tsconfig.json as noEmit + bundler + include: ["src//*", "test//*"]. ed07aec removed it. I ran that config verbatim against this branch and it was clean, apart from local @types/node resolution noise on my machine. If CI failed on it, that is worth recording in the PR body. If it was dropped to keep the diff small, it is the fix for the blocking item.

I'd argue that it's implied in the description (Without doing a heavy refactor of typechecking...) and I created a separate issue for it when I reverted the changes: #135 . I'll try and make this more explicit in the future. The CI failed with it because the files being typechecked depended on artifacts that were not yet available in the CI. I made it a separate issue so it can be thought through in a better context

🔵 followup — README is stale on this exact API
packages/simulator/README.md still shows new MySimulator([...], {...}) (L37), BaseSimulatorOptions (L93, L262, L270), and a four-argument createSimulator<P, L, W, TArgs> that is missing the TContract slot (L59). Pre-existing rather than introduced here, but this PR is about create's contract and the README never documents create

I updated create in the "Extending the Base Simulator" section. The rest of the stale docs should be done separately

LMK if you disagree with any of these points

[0xisk]

LGTM! Thank you @andrew-fleming!