Skip to content

fix: korbit ws version (v1 -> v2)#2498

Merged
haminprk merged 1 commit intomasterfrom
hotfix/korbit/ws-version
Mar 19, 2026
Merged

fix: korbit ws version (v1 -> v2)#2498
haminprk merged 1 commit intomasterfrom
hotfix/korbit/ws-version

Conversation

@haminprk
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue.
Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Deployment

  • Should publish npm package
  • Should publish Docker image

@haminprk haminprk self-assigned this Mar 17, 2026
@haminprk haminprk requested a review from a team as a code owner March 17, 2026 07:28
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

This PR migrates the Korbit WebSocket integration from a v1 user push endpoint to a v2 public endpoint. The subscription payload structure, message format, ticker fields, and utility functions are updated to align with the new API specification.

Changes

Cohort / File(s) Summary
Type Definitions & Configuration
node/pkg/websocketfetcher/providers/korbit/type.go
Updated WebSocket endpoint URL from v1 user push to v2 public. Restructured Raw message format to use Type/Timestamp/Symbol/Snapshot fields. Completely redesigned Ticker struct with new fields (Close, PrevClose, PriceChange, PriceChangePercent, BestAskPrice, BestBidPrice, LastTradedAt). Redefined Subscription with Method/Type/Symbols instead of AccessToken/Event/Data.
Handler Logic & Message Processing
node/pkg/websocketfetcher/providers/korbit/korbit.go
Updated subscription payload construction to use new Method/Type/Symbols format. Added guard to drop control messages with status field. Changed message filtering from Event-based to Type-based check. Updated data extraction to use RawToFeedData instead of DataToFeedData with adjusted error logging.
Utility Functions
node/pkg/websocketfetcher/providers/korbit/utils.go
Renamed DataToFeedData to RawToFeedData with updated parameter from Ticker to Raw. Changed timestamp, volume, and currency pair extraction to use raw.Timestamp, raw.Data.Close/Volume, and raw.Symbol respectively.
Test Updates
node/pkg/websocketfetcher/tests/utils_test.go
Updated TestMessageToStructKorbit test input JSON to match v2 message format with Type/Timestamp/Symbol/Snapshot fields. Updated assertions to validate new field names (Type, Symbol, Close, Volume) instead of previous structure (Event, CurrencyPair, Last).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hoppy hops across the wire,
To v2's heights, we now aspire!
Symbols dance where tokens flew,
Public streams in ticker's hue.
Korbit's pulse beats fresh and bright! 📡✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is an unfilled template with no actual information about the changes, motivation, dependencies, or related issue references. Fill in the description with actual summary of changes, motivation/context, dependencies, related issue number, change type selection, and self-review/test checklist confirmation.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: korbit ws version (v1 -> v2)' clearly and specifically describes the main change: upgrading Korbit WebSocket from v1 to v2.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/korbit/ws-version
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

Tip

CodeRabbit can use your project's `golangci-lint` configuration to improve the quality of Go code reviews.

Add a configuration file to your project to customize how CodeRabbit runs golangci-lint.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
node/pkg/websocketfetcher/tests/utils_test.go (1)

207-245: LGTM - Test updated for v2 schema.

The test correctly validates the new Korbit v2 message structure, checking:

  • Top-level fields: Type, Symbol, Timestamp
  • Nested data fields: Close, Volume

Consider adding test cases for control messages (with "status" field) and invalid symbol formats to improve coverage, though this is optional.

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@node/pkg/websocketfetcher/tests/utils_test.go` around lines 207 - 245, Test
covers Korbit v2 message structure but reviewer suggested adding optional
coverage for control messages and invalid symbols; add two small subtests that
reuse common.MessageToStruct[korbit.Raw]: one that passes a control-message JSON
with a top-level "status" field and validates the parsed struct (e.g., Status or
control-related fields exist) and one that passes an invalid symbol format to
verify MessageToStruct returns an error (or appropriate handling) for
korbit.Raw; locate test "TestMessageToStructKorbit" and add these subtests
alongside it, referencing common.MessageToStruct and the korbit.Raw type so the
new cases exercise control-message parsing and symbol validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@node/pkg/websocketfetcher/tests/utils_test.go`:
- Around line 207-245: Test covers Korbit v2 message structure but reviewer
suggested adding optional coverage for control messages and invalid symbols; add
two small subtests that reuse common.MessageToStruct[korbit.Raw]: one that
passes a control-message JSON with a top-level "status" field and validates the
parsed struct (e.g., Status or control-related fields exist) and one that passes
an invalid symbol format to verify MessageToStruct returns an error (or
appropriate handling) for korbit.Raw; locate test "TestMessageToStructKorbit"
and add these subtests alongside it, referencing common.MessageToStruct and the
korbit.Raw type so the new cases exercise control-message parsing and symbol
validation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1d0fea8b-8a3d-4f7c-a30e-6c0d274ce1a4

📥 Commits

Reviewing files that changed from the base of the PR and between a8d706a and 20fe824.

📒 Files selected for processing (4)
  • node/pkg/websocketfetcher/providers/korbit/korbit.go
  • node/pkg/websocketfetcher/providers/korbit/type.go
  • node/pkg/websocketfetcher/providers/korbit/utils.go
  • node/pkg/websocketfetcher/tests/utils_test.go

@haminprk haminprk merged commit 78763c3 into master Mar 19, 2026
2 checks passed
@haminprk haminprk deleted the hotfix/korbit/ws-version branch March 19, 2026 00:57
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