Skip to content

fix(perps-controller): repair CommonJS build and add close leverage + order latency#9471

Open
abretonc7s wants to merge 7 commits into
mainfrom
fix/perps-controller-cjs-hyperliquid
Open

fix(perps-controller): repair CommonJS build and add close leverage + order latency#9471
abretonc7s wants to merge 7 commits into
mainfrom
fix/perps-controller-cjs-hyperliquid

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Explanation

perps-controller@9.2.1 (#9311) has a broken CommonJS build: dist/services/HyperLiquidClientService.cjs and dist/utils/standaloneInfoClient.cjs emit require("file:///home/runner/work/hyperliquid/hyperliquid/src/mod.ts") — an absolute build-machine path — so any CJS/bundler/Jest consumer throws Cannot find module. Root cause is upstream: @nktkas/hyperliquid (0.33.0+) ships .d.ts with a /// directive (a Deno->npm dnt build artifact); ts-bridge uses that amd-module name as the CJS require() target, leaking the absolute path. The ESM build is unaffected.

This PR:

  1. Fixes the CJS build with a yarn patch on @nktkas/hyperliquid that strips the amd-module directives, applied via the root monorepo resolutions (build-time only). perps-controller's own dependency range stays ^0.33.1 — the patch is deliberately NOT in the package's dependencies, since a published patch: specifier would break every downstream consumer. After rebuild both .cjs files emit require("@nktkas/hyperliquid") with zero file: leaks. Bumping the SDK does not fix this (0.33.1 is latest and still ships the directive; 0.32.2 is the last clean version but predates breaking API changes this package depends on).
  2. — PERPS_POSITION_CLOSE_TRANSACTION.leverage is now the computed effective leverage abs(positionValue)/marginUsed rounded to 1 decimal (guarded for finite values and marginUsed > 0), populated for every close including TP/SL.
  3. — adds optional TrackingData.orderExecutionLatencyMs, emitted as order_execution_latency_ms on the trade terminal event only (scoped to #trackOrderResult, not the shared attribution builder), with a !== undefined guard.

All changes are additive and recorded under ## [Unreleased] in the CHANGELOG (version stays 9.2.1 in this PR per release-cut convention; the next perps-controller release picks them up).

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — N/A, additive/non-breaking

Note

Medium Risk
The yarn patch changes how a critical HyperLiquid SDK resolves in CJS builds (high impact if wrong), but the change is build-time only and narrowly strips bad .d.ts directives; runtime trading/analytics edits are mostly additive with targeted test coverage.

Overview
Fixes perps-controller CommonJS consumers that broke on 9.2.1 because ts-bridge inlined require("file:///home/runner/.../mod.ts") from @nktkas/hyperliquid’s .d.ts amd-module directives. A root monorepo yarn patch (via resolutions) strips those directives so emitted .cjs files use require("@nktkas/hyperliquid"); the package dependency range stays ^0.33.1.

Analytics and types in TradingService / TrackingData: optional orderExecutionLatencyMs on the terminal trade event; an extra partially_filled trade event when an open fill is smaller than requested; close events report effective leverage (|positionValue| / marginUsed, 1 dp) including TP/SL; batch closePositions summary adds number_positions_closed; updateMargin emits a failed Risk Management event when the provider returns { success: false } without throwing; TradeAction adds flip direction literals.

Reviewed by Cursor Bugbot for commit 30ea9ed. Bugbot is set up for automated code reviews on this repo. Configure here.

… order latency

The published 9.2.1 CJS build emits require("file:///home/runner/.../mod.ts")
because @nktkas/hyperliquid ships .d.ts with an amd-module directive carrying an
absolute CI path; ts-bridge uses it as the CJS require target, so any CJS consumer
throws Cannot find module. Fixed via a yarn patch on the SDK applied through the
root monorepo resolutions (build-time only); perps-controller's dependency range
stays ^0.33.1. Upstream: nktkas/hyperliquid#165

Also:
- TAT-3147: close events emit computed effective leverage (abs(positionValue)/marginUsed,
  1 dp), incl. TP/SL closes.
- TAT-3084: add optional TrackingData.orderExecutionLatencyMs, emitted as
  order_execution_latency_ms on the trade terminal event only.

Bumps perps-controller to 9.2.2.
…rnal ticket refs

Move the pending analytics and CommonJS-fix entries from a premature [9.2.2]
section to [Unreleased] and revert the package version to 9.2.1 so the release
check passes (package versions are bumped at release-cut, not in feature PRs).
Remove internal ticket references from the changelog, code comments, and test
names, and apply oxfmt formatting.
The check-changelog action requires user-facing Unreleased entries to link to
the current pull request. Add the PR reference to the added, changed, and fixed
entries.
Mirror the close path on the open-trade path in #trackOrderResult: when a
successful order fills for less than the requested size, emit an additional
PERPS_TRADE_TRANSACTION event with status=partially_filled plus amount_filled
and remaining_amount. Full fills are unchanged. Previously partial fills only
emitted status=executed, hiding the fill's partiality.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 82545e5. Configure here.

Comment thread packages/perps-controller/src/services/TradingService.ts
Comment thread packages/perps-controller/src/services/TradingService.ts
…ing failures

updateMargin only emitted the failed Perp Risk Management analytics event on
the thrown-error path, so a non-throwing { success: false } provider result
silently lost the terminal event. Emit the failed event on that branch too,
with the error message from the result, keeping exactly-once semantics per
operation (success emits executed, non-throwing failure emits failed, thrown
errors emit failed in the catch).
Add flip_long_to_short and flip_short_to_long to the TradeAction union
alongside create_position and increase_exposure. The runtime already forwards
these values verbatim, so consumers no longer need casts when deriving flip
actions.
…mmary

The closePositions batch summary event carried only status, completion_duration
and bulk_action_id, silently dropping the closed count after clients removed
their own close-all summary emission. Add a NUMBER_POSITIONS_CLOSED event
property and emit the successful-close count on the batch summary event.
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