Skip to content

Add smart-money-pnl-leaderboard research command - #558

Merged
gulshngill merged 5 commits into
mainfrom
feat/research-smart-money-pnl-leaderboard
Sep 4, 2026
Merged

Add smart-money-pnl-leaderboard research command#558
gulshngill merged 5 commits into
mainfrom
feat/research-smart-money-pnl-leaderboard

Conversation

@gulshngill

Copy link
Copy Markdown
Contributor

Summary

  • add nansen research smart-money-pnl-leaderboard
  • map chain, timeframe, filter, sort, and pagination options to the public API request
  • document the command and add focused API, dispatch, validation, and coverage tests

Validation

  • npm test -- src/__tests__/research.test.js src/__tests__/coverage.test.js — 88 passed
  • npm test — 2,548 passed, 2 skipped
  • npm run lint
  • git diff --check main...HEAD

@nansen-pr-reviewer

nansen-pr-reviewer Bot commented Sep 3, 2026

Copy link
Copy Markdown

pr-reviewer Summary for #725653d

📝 1 finding

Review completed. Please address the findings below.

Findings by Severity

Severity Count
🔵 Low 1

Review effort: 2/5 (Simple)

Summary

This PR is well-structured and follows established repo patterns throughout. The new smart-money-pnl-leaderboard command, the query-options.js extraction, and the --limit validation fix are all sound. Validation logic is thorough: timeframe allowlist, strict pagination validation, cleanBody properly strips undefined/empty-object fields before the API call, and the filters default ({}) is consistent with every other API method.

Findings (1 low)

src/cli.jsexport statement interleaved between import statements (low)

Lines 16–17 in the diff:

import { buildPagination, parseSort } from './query-options.js';
export { buildPagination, parseSort };
import { resolveAddress, isEnsName } from './ens.js';

The re-export is sandwiched between two import blocks, which is syntactically valid ESM but inconsistent with every other export pattern in this codebase (all other re-exports and named exports appear either co-located with their definitions or after all imports). It won't cause a runtime error and ESLint won't flag it under the current config, but it's surprising to read.

Suggested fix: Move the export { buildPagination, parseSort }; line to after all import statements, or use a single export { buildPagination, parseSort } from './query-options.js'; form to make the re-export intent immediately obvious.


Token usage: 1,362 input, 6,648 output, 1,003,870 cache read, 38,007 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer 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.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

…timeframe-days

- schema.json: add filters, sort, page, and limit to the
  smart-money-pnl-leaderboard entry so agents reading 'nansen schema'
  can discover them
- research.js: reject --timeframe-days values outside 1|7|30|90|180
  with an INVALID_PARAMS error instead of forwarding them to the API

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer 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.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer 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.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

cli.js and commands/research.js previously held separate copies with
diverging semantics. Both now import one canonical implementation that
validates --limit as a positive integer; cli.js re-exports for existing
importers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nansen-pr-reviewer[bot]
nansen-pr-reviewer Bot previously approved these changes Sep 3, 2026

@nansen-pr-reviewer nansen-pr-reviewer 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.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

Resolve conflicts with chain-rank (#555), token-sectors (#556), and
address-premium-labels (#557): keep all direct research subcommands in
the subcommand set, help text, README, schema, and tests.

cli.js and research.js share parseSort from src/query-options.js;
research.js keeps its strict --page validation (main's tests require it)
while the shared buildPagination keeps the lenient clamping the cli
tests require.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@nansen-pr-reviewer nansen-pr-reviewer 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.

Auto-approved

This PR was automatically approved because:

  • Claude recommends approval
  • Claude assessed this as a moderate effort change
  • The effort level is within the auto-approval threshold of 2
  • No high or critical issues were detected
  • Review comment contains non-blocking feedback

If you have any concerns, please request a manual review.

@gulshngill
gulshngill merged commit a1d0dd6 into main Sep 4, 2026
9 checks passed
@gulshngill
gulshngill deleted the feat/research-smart-money-pnl-leaderboard branch September 4, 2026 06:23
gulshngill added a commit that referenced this pull request Sep 4, 2026
Resolve conflicts with chain-rank (#555), token-sectors (#556),
address-premium-labels (#557), and smart-money-pnl-leaderboard (#558):
keep all direct research subcommands in the subcommand set, help text,
README, schema, and tests. cli.js takes the shared query-options import
from main and this branch's researchSub name for the dispatch call.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Sep 4, 2026
gulshngill added a commit that referenced this pull request Sep 4, 2026
Resolve conflicts with chain-rank (#555), token-sectors (#556),
address-premium-labels (#557), smart-money-pnl-leaderboard (#558), and
position-intelligence (#559): keep all direct research subcommands in
the subcommand set, help text, README, schema, and tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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