Skip to content

[codex] Add signal endpoints#204

Open
createMonster wants to merge 1 commit into
masterfrom
codex-get-signal-endpoints
Open

[codex] Add signal endpoints#204
createMonster wants to merge 1 commit into
masterfrom
codex-get-signal-endpoints

Conversation

@createMonster
Copy link
Copy Markdown
Contributor

@createMonster createMonster commented May 18, 2026

Summary

  • Add public signal helpers: san.available_signals(), san.get_signal(signal, ...), and san.get_raw_signals(...).
  • Keep signal access out of san.get(metric, ...) so the first positional argument remains the metric identifier.
  • Support raw signal queries with or without slug / selector for time-windowed raw event fetches.
  • Document the new signal helpers in the README.

Validation

  • pytest san/tests/test_signals.py initially failed before implementation, then passed after adding the wrappers.
  • pytest
  • ruff check .
  • git diff --check
  • Live smoke with SanAPI: available_signals() returned 12 names, get_raw_signals(...) returned 23 rows, and get_signal(...) returned 12 rows for the tested window.

Summary by CodeRabbit

  • New Features

    • Added signal endpoints with three new functions: available_signals(), get_signal(), and get_raw_signals() for retrieving signals and anomaly events.
    • Parameter validation now supports "signals" keyword argument.
  • Documentation

    • Added "Signal endpoints" section to README with usage examples.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

📝 Walkthrough

Walkthrough

Three new signal query endpoints—available_signals(), get_signal(), and get_raw_signals()—are introduced to retrieve signal data from SanBase. The implementation includes GraphQL query construction, result transformation to DataFrames, parameter validation, comprehensive unit tests, and user documentation.

Changes

Signal Query Endpoints

Layer / File(s) Summary
Parameter Validation Setup
san/param_validation.py
The "signals" keyword argument is added to the strict validation allowlist, enabling strict-mode calls to accept this parameter without raising validation errors.
Signal Query Implementation
san/signals.py
A new module provides three query functions: available_signals() queries signal metadata; get_signal(signal, **kwargs) retrieves timeseries data with slug or selector; get_raw_signals(**kwargs) retrieves multi-signal raw data with optional filtering. Helper functions centralize argument transformation and selector/slug validation. All queries construct GraphQL strings and convert results to datetime-indexed DataFrames.
Package Integration
san/__init__.py
The three signal functions are imported and added to __all__ to expose them as public API.
Test Coverage for Signal Endpoints
san/tests/test_signals.py
Five test functions validate endpoint behavior: metadata list retrieval, timeseries DataFrame construction and query structure, required argument enforcement, multi-signal retrieval with selector mapping, and conditional selector query construction.
User Documentation
README.md
A "Signal endpoints" section is added to the Table of Contents and main documentation, describing the three functions with a Python usage example.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • santiment/sanpy#202: Both PRs touch san/param_validation.py's strict-keyword validation logic; this PR extends the allowlist to accept "signals" for signal endpoints.

Suggested reviewers

  • tspenov
  • IvanIvanoff

Poem

🐰 Signals now flow through the Python streams,
Three endpoints weave through API dreams,
DataFrames dance with datetime's grace,
GraphQL queries find their place,
A rabbit hops through tests so bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: adding signal endpoints (get_available_signals, get_signal, get_raw_signals) to the Santiment client library. The title is concise, clear, and specific enough for quick understanding.
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex-get-signal-endpoints

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.

@createMonster createMonster force-pushed the codex-get-signal-endpoints branch 2 times, most recently from da14fe5 to 5dff95f Compare May 18, 2026 06:33
@createMonster createMonster marked this pull request as ready for review May 18, 2026 06:43
Copy link
Copy Markdown

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

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 `@san/sanbase_graphql.py`:
- Line 171: The code currently does a raw kwargs.pop("signal") which raises
KeyError if omitted; replace this with an explicit presence check and raise
SanError instead (e.g., if "signal" not in kwargs: raise SanError("missing
required 'signal' argument") else signal = kwargs.pop("signal")), and ensure
SanError is imported/available in the module so the function that uses signal
(the block referencing kwargs.pop("signal")) returns a stable API error rather
than leaking KeyError.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2d327524-fb94-42cc-af0d-487d2d3b2b5a

📥 Commits

Reviewing files that changed from the base of the PR and between 4caa858 and 5dff95f.

📒 Files selected for processing (6)
  • san/param_validation.py
  • san/query_constants.py
  • san/sanbase_graphql.py
  • san/sanbase_graphql_helper.py
  • san/tests/test_get.py
  • san/transform.py

Comment thread san/sanbase_graphql.py Outdated
@createMonster createMonster force-pushed the codex-get-signal-endpoints branch 2 times, most recently from 282d150 to edfb125 Compare May 18, 2026 07:24
@createMonster createMonster force-pushed the codex-get-signal-endpoints branch from edfb125 to a4cefdb Compare May 18, 2026 08:40
Copy link
Copy Markdown

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

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 `@README.md`:
- Around line 386-401: The example fails because the san module is not imported;
add "import san" at the top of the snippet so calls to san.available_signals(),
san.get_signal(...), and san.get_raw_signals(...) run without NameError; ensure
the import appears before those function calls in the README example.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6ad443d-137e-4afe-a90c-f9499989d31a

📥 Commits

Reviewing files that changed from the base of the PR and between 5dff95f and a4cefdb.

📒 Files selected for processing (5)
  • README.md
  • san/__init__.py
  • san/param_validation.py
  • san/signals.py
  • san/tests/test_signals.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • san/param_validation.py

Comment thread README.md
Comment on lines +386 to +401
```python
signals = san.available_signals()

signal_data = san.get_signal(
"anomaly_total_liquidations",
slug="ethereum",
from_date="2026-01-01",
to_date="2026-05-18",
)

raw_signals = san.get_raw_signals(
signals=["anomaly_total_liquidations"],
from_date="2026-01-01",
to_date="2026-05-18",
)
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make the signal example copy-paste runnable.

This snippet uses san without importing it, so it fails with NameError when run as-is.

Suggested docs fix
 ```python
+import san
+
 signals = san.available_signals()
 
 signal_data = san.get_signal(
     "anomaly_total_liquidations",
🤖 Prompt for 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.

In `@README.md` around lines 386 - 401, The example fails because the san module
is not imported; add "import san" at the top of the snippet so calls to
san.available_signals(), san.get_signal(...), and san.get_raw_signals(...) run
without NameError; ensure the import appears before those function calls in the
README example.

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