feat: add short-interest conviction context - #1498
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds a short-interest data feed to the conviction scoring system. It includes an adapter module to fetch and normalize FINRA/exchange short-interest data, a ChangesShort-interest feed and annotation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SignalsAPI
participant ShortInterestFlow
participant Database
Client->>SignalsAPI: request conviction scores
SignalsAPI->>ShortInterestFlow: short_interest_annotation(ticker, cusip)
ShortInterestFlow->>Database: query short_interest table
Database-->>ShortInterestFlow: latest record or none
ShortInterestFlow-->>SignalsAPI: annotation dict or null
SignalsAPI-->>Client: ConvictionScoreResponse with short_interest fields
sequenceDiagram
participant Scheduler
participant IngestFlow
participant ShortInterestAdapter
participant FINRAEndpoint
participant Database
Scheduler->>IngestFlow: ingest_short_interest_for_issuers(issuers)
IngestFlow->>ShortInterestAdapter: fetch_short_interest(ticker)
ShortInterestAdapter->>FINRAEndpoint: HTTP GET short-interest data
FINRAEndpoint-->>ShortInterestAdapter: JSON/CSV payload
ShortInterestAdapter-->>IngestFlow: normalized rows
IngestFlow->>Database: upsert_short_interest(rows)
Database-->>IngestFlow: inserted count
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fixed the concrete Gate failure in a9b023d by documenting FINRA_SHORT_INTEREST_URL in .env.example. Validation: 26 focused tests passed (env coverage, short-interest flow, signals API, schema); focused Ruff and git diff --check passed. Fresh CI is now required; no review threads were open. |
a9b023d to
e5e36eb
Compare
|
Closer lane: stack unwound after #1497 merged. What changed
Validation on the rebased head
Pre-existing main defect found while validating (not from this PR, no action taken here) Fresh CI and CodeRabbit review are now running against |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@adapters/short_interest.py`:
- Around line 40-51: Validate the normalized date produced by _date_value before
persistence, and have normalize_short_interest_row reject rows when report_date
is missing or not a real calendar date such as 2026-99-99. Add tests covering
both missing and invalid report_date inputs, while preserving valid date
normalization.
- Line 123: Update the URL selection in the short-interest adapter around the
environment lookup to fall back to DEFAULT_FINRA_SHORT_INTEREST_URL when
FINRA_SHORT_INTEREST_URL is unset or empty. Retain the empty documented override
in .env.example at line 76; it requires no direct change because the adapter
fallback corrects its behavior.
- Around line 63-72: Update the field selection in the short-interest
normalization flow around _finite_float so each alias chain selects the first
present value rather than the first truthy value, preserving numeric zero for
short_interest and short_interest_pct (and the related fields). Add a test
covering short_interest=0 and short_interest_pct=0 that verifies the row is
retained and normalized correctly.
In `@alembic/versions/018_short_interest.py`:
- Line 20: The metric_id column definition uses sa.BigInteger() which causes
SQLite to store NULL for auto-generated values instead of proper identifiers.
Update the column type for metric_id from sa.BigInteger() to
sa.BigInteger().with_variant(sa.Integer(), "sqlite") to ensure SQLite uses the
appropriate INTEGER type for auto-increment generation while maintaining
compatibility with other database backends.
In `@etl/short_interest_flow.py`:
- Around line 109-120: Restrict the try/except in the short-interest processing
flow to only the fetch_short_interest(ticker, fetcher=fetcher) call. Keep fetch
failures appended to errors and handled by raise_on_fetch_error, but move row
normalization and upsert_short_interest(conn, rows) outside the guarded block so
database or persistence errors propagate unchanged.
- Around line 149-157: The current query uses OR to join ticker and cusip
conditions, which can return a newer record for a different issuer when both
identifiers are present. Replace the logic that appends both conditions to the
clauses list with a priority-based approach where cusip takes precedence: when
cusip is available, add only the cusip clause to clauses and skip the ticker
clause; when cusip is not available but ticker exists, add only the ticker
clause. This ensures the query uses CUSIP as the primary identifier and falls
back to ticker only when CUSIP is absent. Additionally, add a regression test
that verifies the query returns the correct short-interest data when both ticker
and cusip rows exist with different dates or values.
- Line 143: Remove the ensure_short_interest_table(conn) call from
short_interest_annotation so the annotation read path performs no schema
initialization or commit. Leave schema setup to the migration or ingestion
initialization flow, while preserving the existing annotation logic.
In `@tests/test_short_interest_flow.py`:
- Around line 44-52: Add an idempotent-upsert regression test alongside
test_ingest_stubbed_short_interest_and_annotation that ingests the same ticker,
report date, and source twice with updated values, then verifies the database
retains exactly one row containing the latest values. Reuse the existing
short_interest_flow ingestion and annotation helpers and preserve the current
successful single-ingest assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ef0de1d-16c5-43e4-8e99-7a828a87e77d
📒 Files selected for processing (8)
.env.exampleadapters/short_interest.pyalembic/versions/018_short_interest.pyapi/signals.pyetl/short_interest_flow.pyschema.sqltests/test_short_interest_flow.pytests/test_signals_api.py
|
Runner dispatch state for codex on PR #1498. Do not edit. |
🤖 Keepalive Loop StatusPR #1498 | Agent: Codex | Iteration 0/12 Current State
Last Codex Run
To retry immediately:
Or wait for the next successful Gate run to automatically retry. 🔍 Failure Classification| Error type | infrastructure |
|
Keepalive Work Log (click to expand)
|
Rejects rows without a real calendar report_date, preserves zero short-interest values, treats a blank FINRA_SHORT_INTEREST_URL as unset, gives metric_id a SQLite INTEGER variant, keeps DB write errors out of the fetch-failure path, removes DDL from the annotation read path, and prefers CUSIP over a reassigned ticker.
The runner writes langsmith-fleet-worker-attempt.json to the repo root, so keepalive committed it into this PR (the same artifact was removed from #1497). .gitignore only covered artifacts/langsmith/.
|
Closer lane: all 8 CodeRabbit findings addressed in
Validation: Also removed: keepalive had committed the generated Note for the next pass: CodeRabbit reported "Review limit reached … next review available in 40 minutes" plus an organization usage spending cap at 14:51Z, so this push may not get an automatic incremental review; trigger |
|
You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 21 minutes. |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
DisagreementNo major disagreements detected. Unique Insights
🔍 LangSmith Traces |
|
Workflow state fingerprint for Agents Verifier. Do not edit. |
Closes #1470
Summary
Scope boundary
mainafter feat: add cosine similarity crowding alerts #1497 merges.Validation
pytest -q tests/test_short_interest_flow.py tests/test_signals_api.py tests/test_schema.pyruff check adapters/short_interest.py etl/short_interest_flow.py api/signals.py tests/test_short_interest_flow.py tests/test_signals_api.py alembic/versions/018_short_interest.pyblack --check adapters/short_interest.py etl/short_interest_flow.py api/signals.py tests/test_short_interest_flow.py tests/test_signals_api.py alembic/versions/018_short_interest.pypython scripts/check_dialect_portability.pyalembic heads->018 (head)Summary by CodeRabbit
New Features
Bug Fixes
Tests