feat(signal): add SignalManager to Python SDK

[RapidPoseidon]

Summary

Adds SDK support for the new Signal Service:

• RapidataSignalManager exposed as client.signals (CRUD entry point: create, get, list, get_run).
• RapidataSignal with lifecycle methods (pause, resume, delete, trigger, update), get_runs / get_run, and a blocking wait_for_next_run(timeout, poll_interval) helper that returns the first new terminal run.
• SignalRun dataclass with is_terminal / succeeded convenience properties.

Implementation

Built on the autogenerated SignalApi — the signal service is already part of the generated OpenAPI client on main, so this PR is just the hand-written manager / entity / run layer plus its wiring. The manager and entity call openapi_service.signal.signal_api.* through the same RapidataApiClient wrapper as every other manager, so 4xx/5xx are converted to RapidataError automatically. (An earlier revision of this branch shipped a raw-HTTP stopgap and regenerated the client itself; both are now obsolete and have been dropped — this branch is rebased onto main.)

The public surface follows the real backend contract:

• create returns only {id, nextRunAt}, so RapidataSignalManager.create() fetches the full signal before returning it.
• trigger spawns the run asynchronously and returns only the signal id, so RapidataSignal.trigger() returns None; use wait_for_next_run() to observe the resulting run.
• SignalRun omits owner_id / owner_mail / created_at — the run payload doesn't carry them.
• list() exposes page / page_size (the endpoint already scopes results to the caller's own + public signals).

Test plan

• python -c "from rapidata import RapidataClient; print('OK')"
• python -c "from rapidata.rapidata_client.signal import RapidataSignalManager, RapidataSignal, SignalRun; print('OK')"
• python -c "from rapidata import RapidataSignal, RapidataSignalManager, SignalRun; print('OK')"
• pyright src/rapidata/rapidata_client -> 0 errors

🔗 Session: https://session-b2a2bd76.poseidon.rapidata.internal/

[claude]

Code Review — feat(signal): add SignalManager to Python SDK

Overview

This PR adds a RapidataSignalManager to the SDK, exposing a new Signal Service via a temporary raw-HTTP layer (_signal_http.py) until the OpenAPI client is regenerated. The design is clean and follows the existing manager/entity pattern well. A few issues below worth addressing before merge.

---

Bugs / Correctness

wait_for_next_run — misleading comment and potentially wrong return value
In rapidata_signal.py the inline comment says "the earliest new terminal — return that one (the next to finish)", but terminal[-1] on a descending-sorted list is actually the oldest new terminal run, not the one that finished most recently. If the caller just called trigger() and wants the result of that specific run, this can silently return a stale run. Consider returning terminal[0] (the newest) or clarifying the intent.

# Current
return terminal[-1]   # oldest new terminal (confusing intent)

# Likely intended
return terminal[0]    # newest terminal run (most recently started that finished)

wait_for_next_run — client/server clock skew
started_waiting = datetime.now(timezone.utc) is a local client timestamp. r.started_at comes from the server. If there's any clock skew (even a few seconds), runs that fired right around wait_for_next_run() being called can be filtered out permanently. A safer approach is to snapshot the run IDs present at call time and look for newly-appearing ones instead:

seen_ids = {r.id for r in self.get_runs(page=1, page_size=20)}
# then look for runs whose id is not in seen_ids

wait_for_next_run — hardcoded page_size=20
If more than 20 runs arrive between polls (e.g. after a burst of trigger() calls), new terminal runs will be silently missed. At minimum, expose page_size as a parameter or use the same value as the default in get_runs.

---

Style / Convention Violations

Double-underscore __openapi_service in RapidataSignalManager
Every other manager in the codebase uses self._openapi_service (single underscore). RapidataSignalManager uses self.__openapi_service (name-mangled). This is inconsistent and will cause AttributeError if anything accesses it via the base name. Change to _openapi_service to match the rest.

# All other managers
self._openapi_service = openapi_service  # ✓

# Signal manager — should match
self.__openapi_service = openapi_service  # ✗

Import inside function body (_signal_http.py, line ~55)
from urllib.parse import urlencode is imported inside the if params: block. Per PEP 8 and project conventions, imports belong at the top of the file.

Missing __repr__ on SignalRun
All other entity classes define both __str__ and __repr__ (with __repr__ delegating to __str__). SignalRun only defines __str__, so repr() falls back to the verbose dataclass default.

---

Missing Items (per CLAUDE.md)

No documentation update
CLAUDE.md states: "when updating any interfaces make sure you update the docs and examples." No docs changes are included in this diff. Even a brief entry in the relevant docs page for client.signals would satisfy this.

No automated tests
The test plan is entirely manual. While the temporary HTTP layer makes pure-unit tests harder to write, at minimum SignalRun._from_api, _parse_datetime (especially the Z-suffix normalization), and wait_for_next_run timeout logic can be unit-tested without network access.

No type annotation for self.signals in rapidata_client.py
All other public attributes on RapidataClient are typed. self.signals has no annotation:

# Add the annotation alongside the assignment
self.signals: RapidataSignalManager = RapidataSignalManager(
    openapi_service=self._openapi_service
)

---

Minor

• update() cannot clear description (set it back to None) — the if description is not None guard prevents it. If unsetting is a valid use-case, consider a sentinel default (e.g. _UNSET = object()).
• trigger() is the only mutating lifecycle method without a managed_print call. pause(), resume(), and delete() all print confirmation. Consider adding one for consistency.
• The _parse_datetime / _parse_optional_datetime helpers are private (underscore-prefixed) but are re-exported across module boundaries inside the same package. If they are needed in rapidata_signal.py, moving them to a shared _utils.py inside the signal package would keep the private API cleaner.

---

Summary

Severity	Count
Bug / Correctness	3
Convention violation	3
Missing required items (CLAUDE.md)	3
Minor	3

The overall structure is solid. Fix the __openapi_service naming, the wait_for_next_run return-value / clock-skew issues, and add the docs update before merging.

[LinoGiger]

this should use the generations and not just send direct api requests

[claude]

Code Review

This PR adds a clean Signal service layer on top of the already-generated SignalApi client. The three-file structure (manager / entity / run dataclass) follows the existing pattern well, and the wait_for_next_run helper is a thoughtful addition. A few correctness and convention issues below.

---

Findings (most severe first)

1. wait_for_next_run returns the wrong run — terminal[-1] is the oldest, not the most recent

src/rapidata/rapidata_client/signal/rapidata_signal.py line 282

get_runs is called with sort_descending=True, so runs is ordered newest-first by started_at. After filtering, terminal preserves that order, so terminal[-1] is the oldest-started terminal run. The docstring promises "The first new run to reach a terminal status" and the primary use-case is calling trigger() then waiting — in that case you want the most recently started terminal run, i.e. terminal[0].

The inline comment ("the last terminal in the list is the earliest new terminal — return that one (the next to finish)") is also misleading: earliest-to-start is not the same as first-to-finish when runs can complete out of order.

Fix: change return terminal[-1] to return terminal[0].

---

2. Timezone-naive started_at vs timezone-aware started_waiting crashes the poll loop

src/rapidata/rapidata_client/signal/rapidata_signal.py line 277

started_waiting = datetime.now(timezone.utc) is always timezone-aware. r.started_at comes from QuerySignalRunsEndpointOutput.started_at: datetime — LazyValidatedModel has no explicit timezone-aware config, so if the backend ever returns a timestamp without a UTC offset (e.g. "2024-01-01T12:00:00" rather than "...Z"), Pydantic stores a naive datetime and the >= comparison on line 277 raises TypeError: can't compare offset-naive and offset-aware datetimes, silently killing the poller.

Fix: Normalize started_at in SignalRun._from_api to always be timezone-aware, or add a UTC-assumption fallback in the comparison.

---

3. update() accepts any interval_seconds with no validation; create() enforces the wrong floor

src/rapidata/rapidata_client/signal/rapidata_signal_manager.py line 53 and rapidata_signal.py line 167

create() guards interval_seconds <= 0 but its own docstring documents "Minimum 60". The real domain constraint is never enforced, so create(..., interval_seconds=1) silently proceeds until the API rejects it with an opaque error.

update() has no guard at all for interval_seconds — a caller passing 0 or -1 gets a backend error instead of a clear SDK ValueError.

Fix: In both create() and update(), validate interval_seconds against the documented minimum (60) and raise ValueError with a clear message.

---

4. Missing documentation and example — CLAUDE.md violation

src/rapidata/rapidata_client/signal/ (all new files)

CLAUDE.md states: "when updating any interfaces make sure you update the docs and examples". RapidataSignal, RapidataSignalManager, and SignalRun are all exported from the top-level rapidata package, but no narrative doc page under docs/ and no example script were added.

---

Minor notes

• page_size=20 hardcoded in wait_for_next_run (line 276): if more than 20 new runs start before any reach a terminal state, the poller only checks page 1 and could time out even though a completed run exists on page 2+. Unlikely in practice, but worth a comment acknowledging the assumption.

• SignalRun module-level import in manager (rapidata_signal_manager.py line 7): SignalRun is only used as a return-type annotation and the file already has from __future__ import annotations. Moving it under TYPE_CHECKING would match the codebase convention.

• Duplicate signal_run_run_id_get + SignalRun._from_api call: RapidataSignalManager.get_run and RapidataSignal.get_run both call the same endpoint and construct a SignalRun independently. A shared private helper on the service layer would keep the deserialization path in one place.

[claude]

Code Review

This PR adds a clean SDK layer for the new Signal Service — RapidataSignalManager, RapidataSignal, and SignalRun — following the same patterns used by the rest of the SDK. The documentation is thorough and the tracing/logging hooks are consistent with other managers. A few issues worth addressing before merge:

---

🔴 create() orphans the signal ID if the follow-up get() fails

src/rapidata/rapidata_client/signal/rapidata_signal_manager.py:80

created = self._openapi_service.signal.signal_api.signal_post(...)
# The create endpoint only echoes the id; fetch the full signal to populate.
return self.get(created.id)   # ← if this raises, created.id is gone

The signal is created on the backend, but created.id is never stored or logged before being passed inline to self.get(). If the second network call fails (transient error, propagation delay, etc.), the exception surfaces with no trace of the ID — the signal is permanently orphaned and the caller has no way to recover it.

Fix: log or capture created.id before the follow-up fetch, e.g.:

signal_id = created.id
logger.debug("Signal created with id '%s', fetching full record", signal_id)
return self.get(signal_id)

This doesn't require error handling — just ensuring the ID appears in logs so it's recoverable after a failure. The job manager avoids this issue entirely by building the entity directly from the create response without a second call.

---

🟡 interval_seconds guard validates > 0 but the documented minimum is 60

src/rapidata/rapidata_client/signal/rapidata_signal_manager.py:53

if interval_seconds <= 0:
    raise ValueError("interval_seconds must be positive")

The docstring (line 44) says "Minimum 60" and the OpenAPI schema carries the same constraint — but the guard only rejects zero and negatives. Values 1–59 pass the SDK check and reach the backend, which returns a 400. The same gap exists in update(), which has no validation on interval_seconds at all.

Fix:

if interval_seconds < 60:
    raise ValueError("interval_seconds must be at least 60")

Apply the same check (or a shared helper) in update().

---

🟡 Timezone mismatch in wait_for_next_run may raise TypeError

src/rapidata/rapidata_client/signal/rapidata_signal.py:277

started_waiting = datetime.now(timezone.utc)   # timezone-aware
# ...
candidates = [r for r in runs if r.started_at >= started_waiting]

r.started_at is typed as plain datetime in both API models (GetSignalRunByIdEndpointOutput, QuerySignalRunsEndpointOutput) — no AwareDatetime annotation, no model-level config enforcing timezone-awareness. If the backend returns timestamps without a timezone suffix (e.g., "2026-06-25T10:00:00" instead of "...Z"), Pydantic stores a naive datetime and the comparison raises TypeError: can't compare offset-naive and offset-aware datetimes, crashing the entire wait_for_next_run call.

Fix: normalize started_at before comparing, or add an AwareDatetime annotation to the generated models. A defensive approach in the SDK:

def _ensure_aware(dt: datetime) -> datetime:
    return dt if dt.tzinfo is not None else dt.replace(tzinfo=timezone.utc)

candidates = [r for r in runs if _ensure_aware(r.started_at) >= started_waiting]

---

🟡 wait_for_next_run silently ignores runs beyond page 20

src/rapidata/rapidata_client/signal/rapidata_signal.py:276

runs = self.get_runs(page=1, page_size=20, sort_descending=True)
candidates = [r for r in runs if r.started_at >= started_waiting]
terminal = [r for r in candidates if r.is_terminal]
if terminal:
    return terminal[-1]

If more than 20 runs start after started_waiting (possible for short-interval signals, burst testing, or long waits), only the 20 newest are examined — earlier terminal runs on page 2+ are silently skipped. The method either times out or returns a later terminal run than intended.

Fix: check the paged response's total field and page further if needed, or use a server-side filter on startedAt if the API supports it.

---

🔵 signal.update(description=None) alone cannot clear the description

src/rapidata/rapidata_client/signal/rapidata_signal.py:179

if name is None and description is None and interval_seconds is None:
    return self  # skip API call

Calling signal.update(description=None) in isolation hits this early return and makes no API call — so there is no way to clear a description without simultaneously changing another field. The workaround is signal.update(name=signal.name, description=None), which bypasses the guard. The docstring and API schema should note this limitation explicitly.

---

Minor nit: _TERMINAL_STATUSES type annotation

src/rapidata/rapidata_client/signal/signal_run.py:21

_TERMINAL_STATUSES: frozenset[str] = frozenset({"Completed", "Failed", "Skipped"})

Typing this as frozenset[SignalRunStatus] would let pyright catch future typos in the set and tighten the narrowing on is_terminal.

---

Overall this is a well-structured addition that follows the SDK's patterns cleanly. The two confirmed bugs (orphaned signal ID and weak interval validation) are worth fixing before shipping; the timezone and pagination issues are worth at least a comment or defensive guard.

[claude]

Code Review

This PR adds a well-structured Signal service layer: RapidataSignalManager, RapidataSignal, and a frozen SignalRun dataclass. The pattern is consistent with the rest of the SDK — the SignalService correctly threads RapidataApiClient so 4xx/5xx errors surface as RapidataError, and the lazy-initialisation pattern matches every other service. Below are the issues worth addressing before merge.

---

1. wait_for_next_run — timezone-naive/aware comparison (PLAUSIBLE crash)

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 279

started_waiting = datetime.now(timezone.utc)   # timezone-aware
...
candidates = [r for r in runs if r.started_at >= started_waiting]

SignalRun.started_at is typed as plain datetime (not AwareDatetime) in both QuerySignalRunsEndpointOutput and GetSignalRunByIdEndpointOutput. If the backend ever returns an ISO 8601 string without a timezone offset, Pydantic v2 will parse it as a naive datetime and the >= comparison raises TypeError: can't compare offset-naive and offset-aware datetimes. The model should use Pydantic's AwareDatetime for started_at, or the comparison should normalise tzinfo defensively.

---

2. wait_for_next_run — returns the oldest new run, not the one just triggered (logic error)

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 284

return terminal[-1]

get_runs is sorted descending by started_at, so terminal[0] is the most-recently-started run and terminal[-1] is the oldest. The docstring says "The first new run to reach a terminal status"; after a trigger() call the user expects the run they just triggered — the newest one, i.e. terminal[0]. In the common single-trigger case there is only one entry so it doesn't matter, but when a concurrent scheduled run also completes between polls, this returns the wrong one. The comment's reasoning ("earliest terminal = next to finish") conflates start order with finish order and is not reliable.

---

3. wait_for_next_run — sleep can overshoot timeout by a full poll_interval

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 286–290

if monotonic() >= deadline:
    raise TimeoutError(...)
sleep(poll_interval)

The deadline is checked before sleeping. If no terminal run is found and the deadline has not been reached, the function sleeps poll_interval seconds before rechecking. The effective maximum wait is therefore timeout + poll_interval, not timeout. With default values this is 300 + 5 = 305 s — probably fine — but callers who pass timeout=5, poll_interval=30 will wait 30 s instead of 5. A simple fix is to sleep min(poll_interval, max(0, deadline - monotonic())).

---

4. trigger() discards the queued run ID from the API response

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 438–441

def trigger(self) -> None:
    ...
    self._openapi_service.signal.signal_api.signal_signal_id_trigger_post(self.id)

signal_signal_id_trigger_post returns a TriggerSignalEndpointOutput whose id is the newly queued run's ID. This is discarded. Because the run ID is lost, wait_for_next_run has to identify the new run by timestamp comparison (started_at >= started_waiting), which is fragile: a scheduled run that fires at the same moment will also match. Returning the run ID from trigger() (or at minimum storing it internally) would allow wait_for_next_run to match on run_id instead of wall-clock time.

---

5. wait_for_next_run — hard-coded page_size=20 silently misses runs

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 278

runs = self.get_runs(page=1, page_size=20, sort_descending=True)

If more than 20 runs have started since started_waiting (e.g. in a load-test scenario or if the poll interval is long), a terminal run that falls outside the first page is never seen and the call times out. Consider either paginating until the oldest returned run pre-dates started_waiting, or exposing page_size as a parameter with a sensible default.

---

Minor

• update() cannot clear a description (rapidata_signal.py, line 460): the early-return guard treats description=None as "leave unchanged". There is no way to clear an existing description through the SDK. At minimum, document this limitation in the docstring.
• AudienceAudienceIdJobsGetJobIdParameter import is inside the function body (rapidata_signal_manager.py, line 114): this cross-module dependency is invisible at the module level. The other managers import it at the top of the file — following that convention would make the dependency visible.

---

Overall the implementation is solid and the architecture matches the rest of the SDK well. The most actionable items are #1 (potential crash), #2 (wrong run returned), and #3 (timeout overshoot).

[claude]

Code Review

This PR adds the Signal Service layer to the Python SDK — a RapidataSignalManager + RapidataSignal pair that wraps the already-generated SignalApi for creating, controlling, and observing recurring labeling schedules. The architecture follows established SDK patterns (lazy OpenAPIService property, RapidataApiClient error handling, TYPE_CHECKING imports), and the documentation is clear and well-structured. A few issues need attention before merging.

---

Confirmed bugs

src/rapidata/__init__.py — SignalRun is missing from exports

The PR description's test plan includes:

python -c "from rapidata import RapidataSignal, RapidataSignalManager, SignalRun; print('OK')"

SignalRun does not exist anywhere in the SDK — no file defines it. The test plan import will raise ImportError on the third name. The PR description mentions SignalRun as a public dataclass with is_terminal/succeeded properties, but it was not implemented. Either implement it (wrapping QuerySignalRunsEndpointOutput/GetSignalRunByIdEndpointOutput) or remove it from the test plan and description. Currently get_jobs() silently discards all run-level metadata (status, trigger source, start time) and returns RapidataJob objects directly.

---

src/rapidata/rapidata_client/signal/rapidata_signal.py line ~254 — timezone-naive vs. timezone-aware TypeError in wait_for_next_job

started_waiting = datetime.now(timezone.utc)   # aware
...
if run.started_at >= started_waiting            # run.started_at may be naive

run.started_at is typed as plain datetime in QuerySignalRunsEndpointOutput with no timezone enforcement. Pydantic v2 will deserialize it as a naive datetime if the backend omits a timezone suffix (e.g. "2024-06-01T10:00:00" rather than "2024-06-01T10:00:00Z"). In that case this comparison raises TypeError: can't compare offset-naive and offset-aware datetimes on the first poll, crashing wait_for_next_job entirely. Suggested fix:

def _ensure_aware(dt: datetime) -> datetime:
    return dt if dt.tzinfo is not None else dt.replace(tzinfo=timezone.utc)

if _ensure_aware(run.started_at) >= started_waiting and run.audience_job_id

---

Plausible issue

src/rapidata/rapidata_client/signal/rapidata_signal.py line ~248 — hardcoded page_size=20 in poll loop can miss jobs

runs = self._openapi_service.signal.signal_api.signal_signal_id_run_get(
    self.id, page=1, page_size=20, sort=["-started_at"]
).items

If more than 20 runs start between two polls (possible for very high-frequency signals with a longer poll_interval), the oldest fresh runs fall off page 1 and wait_for_next_job misses them, eventually raising TimeoutError despite the job existing. The loop should either paginate until it reaches a run older than started_waiting, or expose page_size as a parameter.

---

Minor issues

docs/signals.md line ~107 vs rapidata_signal.py line ~207 — timeout default mismatch

The docs example passes timeout=600 but the implementation default is timeout=300. Not wrong (the example is a valid explicit call), but a reader inferring the default from the example will be surprised. Align them.

src/rapidata/rapidata_client/signal/rapidata_signal.py lines 59–79 — each property fires a separate HTTP call

name, description, interval_hours, next_run_at, last_run_at, and is_paused each call _latest() independently. Accessing all six properties issues six round-trips. The docs describe the signal as a "live handle" so this is intentional, but a snapshot() or refresh() convenience method would let callers avoid the N-call penalty when they need multiple fields at once.

[claude]

Code Review

This PR adds a clean signal management layer on top of the autogenerated SignalApi: a RapidataSignalManager wired into the client, a RapidataSignal entity with lifecycle methods, and a wait_for_next_job polling helper. The service/manager/entity structure follows existing conventions well. A few correctness issues in wait_for_next_job are worth addressing before merge.

---

Findings (most severe first)

1. wait_for_next_job silently skips in-progress runs, causing spurious TimeoutError

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 254

The filter if run.audience_job_id drops any run where audience_job_id is None. The generated model documents exactly this: "null until orders confirms creation". Between the trigger firing and the orders service confirming the job, every poll skips the run silently. If provisioning takes longer than timeout, a TimeoutError is raised even though the trigger succeeded. Fix: track run IDs matching started_at >= started_waiting across polls and wait for their audience_job_id to become non-null, rather than discarding them.

---

2. wait_for_next_job ignores trigger_source and may return the wrong job

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 257

The generated run model has a trigger_source field ("manual" vs "scheduler"). The polling loop collects all new runs regardless of source. If a scheduled firing coincides with a manual trigger() call, fresh_job_ids[-1] (oldest in a newest-first list) may be the scheduled run rather than the triggered one. Filtering by trigger_source == "manual" (or returning fresh_job_ids[0], the newest) would be more correct for the documented use case.

---

3. started_at >= started_waiting crashes if the API returns a timezone-naive datetime

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 254

started_waiting = datetime.now(timezone.utc) is always timezone-aware. If the backend omits the UTC suffix from startedAt, pydantic deserializes it as a naive datetime and the comparison raises TypeError. No guard exists. Adding .replace(tzinfo=timezone.utc) on run.started_at defensively would prevent this.

---

4. Hard-coded page_size=20 in wait_for_next_job can miss runs under high firing rate

src/rapidata/rapidata_client/signal/rapidata_signal.py, line 248

The polling loop always fetches exactly one page of 20 runs. If more than 20 firings accumulate between two poll cycles, runs beyond the first page are never checked and fresh_job_ids stays empty until TimeoutError. A server-side started_after timestamp filter (if the API supports it) would be the clean fix; alternatively, paginate until started_at < started_waiting.

---

Minor / design notes

• Each mutable property makes a separate HTTP call. name, description, interval_hours, next_run_at, last_run_at, and is_paused each independently call _latest(). Code like f"{signal.name}: paused={signal.is_paused}" fires two round-trips reading potentially inconsistent server state. A refresh() method that fetches once and caches all fields atomically would be both faster and safer.

• SignalRun described in the PR body is not implemented. The description mentions a SignalRun dataclass with is_terminal/succeeded and get_runs()/get_run() methods. None of these exist in the diff. Fine if deferred, but the PR description should be updated; callers currently have no SDK-level way to inspect skipped or failed runs (silently dropped by if run.audience_job_id).

• get_jobs() fetches each job sequentially — N runs = N serial HTTP calls. For page_size=20 that is up to 20 round-trips. Worth noting for future optimization.

[claude]

Code Review — PR #598: feat(signal): add SignalManager to Python SDK

This PR adds a clean new RapidataSignalManager / RapidataSignal layer on top of the existing autogenerated SignalApi, following the same manager/entity pattern used by orders, jobs, and audiences. The service wiring and lazy-initialization pattern are consistent with the rest of the codebase. A few issues worth addressing before the next release:

---

🔴 High — Potential TypeError crash in wait_for_next_job

src/rapidata/rapidata_client/signal/rapidata_signal.py, line ~254

started_waiting = datetime.now(timezone.utc)   # timezone-aware
...
if run.started_at >= started_waiting ...        # run.started_at may be naive

started_waiting is timezone-aware (datetime.now(timezone.utc)). The OpenAPI-generated model types started_at as plain datetime with no timezone guarantee. If the backend sends a naive timestamp (no UTC offset in the JSON), Pydantic deserializes it as a naive datetime, and this comparison raises TypeError: can't compare offset-naive and offset-aware datetimes — crashing every call to wait_for_next_job. Fix: strip the tzinfo for comparison (started_waiting_naive = datetime.utcnow()) or ensure the generated model uses AwareDatetime.

---

🟡 Medium — wait_for_next_job returns wrong job after trigger()

src/rapidata/rapidata_client/signal/rapidata_signal.py, lines ~251–257

fresh_job_ids = [
    run.audience_job_id
    for run in runs          # newest-first (sort=["-started_at"])
    if run.started_at >= started_waiting and run.audience_job_id
]
if fresh_job_ids:
    return job_manager.get_job_by_id(fresh_job_ids[-1])   # oldest of the fresh ones

The list is in newest-first order, so fresh_job_ids[-1] is the oldest new firing — the comment confirms this. But after calling trigger(), the triggered job is the most recent firing (fresh_job_ids[0]). If the signal also fired on its regular schedule between the trigger() call and the first poll, wait_for_next_job returns the scheduled job rather than the triggered one. The primary use-case in the docs (signal.trigger(); job = signal.wait_for_next_job()) silently returns the wrong job. Consider returning fresh_job_ids[0] (newest fresh job) instead.

---

🟡 Medium — Each mutable property is a separate HTTP round-trip

src/rapidata/rapidata_client/signal/rapidata_signal.py, lines ~58–80

@property
def name(self) -> str:
    return self._latest().name       # GET /signal/{id}

@property
def description(self) -> str | None:
    return self._latest().description  # another GET /signal/{id}

# ... same for interval_hours, next_run_at, last_run_at, is_paused

Six properties, each calling _latest() → signal_signal_id_get(). Accessing two or three of them in a single display statement makes 2–3 serial HTTP calls. This is documented in the class docstring, but the rest of the SDK uses explicit get_status() methods (e.g. RapidataJob.get_status()) to make the network cost visible to callers. Consider either exposing a refresh() method that caches a snapshot of all mutable fields, or converting the live properties to an explicit get_status() / get_details() method consistent with other entity classes.

---

🟡 Medium — Timeout can overshoot by poll_interval seconds

src/rapidata/rapidata_client/signal/rapidata_signal.py, lines ~259–263

if monotonic() >= deadline:
    raise TimeoutError(...)
sleep(poll_interval)          # sleep comes AFTER the check

Wait — looking again at the code order: the deadline check is followed by sleep. So when the poll returns empty results, the code checks the deadline first, and if not expired, then sleeps. This means the last sleep happens before the check — the actual elapsed time before TimeoutError can be up to timeout + poll_interval. With defaults (timeout=300, poll_interval=5) this is a ~2% overshoot, but with timeout=5, poll_interval=30 the function blocks for 30 s past its deadline. Fix: cap the sleep duration: sleep(min(poll_interval, max(0, deadline - monotonic()))).

---

🔵 Low — update(description=None) cannot clear the description field

src/rapidata/rapidata_client/signal/rapidata_signal.py, lines ~144–146

if name is None and description is None and interval_hours is None:
    return self   # early return — no API call

This guard treats description=None as "leave unchanged", making it impossible to explicitly clear a description to None. A caller who writes signal.update(description=None) expecting to clear the field is silently ignored. If the API supports null as a valid description value, a sentinel (e.g. a _UNSET object) is needed to distinguish "not provided" from "clear to null".

---

🔵 Low — SignalRun mentioned in PR description but not implemented

The PR summary lists SignalRun as part of the public surface (with is_terminal/succeeded properties and get_runs/get_run on RapidataSignal). None of this exists in the merged code — get_jobs() resolves run audience_job_id fields directly to RapidataJob objects with no intermediate SignalRun wrapper, and callers have no way to inspect run metadata (status, whether a run was skipped, trigger source). If SignalRun was intentionally cut, the PR description should be updated; if it was accidentally dropped, it's a missing feature.

---

Summary

Severity	File	Issue
🔴 High	rapidata_signal.py:254	Tz-naive/aware comparison → TypeError crash in wait_for_next_job
🟡 Medium	rapidata_signal.py:256	fresh_job_ids[-1] returns oldest fresh job, not most recent — wrong after trigger()
🟡 Medium	rapidata_signal.py:58–80	6 properties = 6 HTTP calls; no snapshot/batching option for callers
🟡 Medium	rapidata_signal.py:261	Timeout can overshoot by poll_interval seconds
🔵 Low	rapidata_signal.py:144	Cannot clear description to None via update()
🔵 Low	__init__.py	SignalRun promised in PR description but absent from public API