Skip to content

feat(job): expose estimated and actual cost on jobs#658

Merged
LinoGiger merged 5 commits into
mainfrom
feat(job)/expose-cost-estimate-and-actual-cost
Jul 8, 2026
Merged

feat(job): expose estimated and actual cost on jobs#658
LinoGiger merged 5 commits into
mainfrom
feat(job)/expose-cost-estimate-and-actual-cost

Conversation

@RapidPoseidon

Copy link
Copy Markdown
Contributor

What

Adds cost properties to the SDK's job objects, backed by the cost-estimate and billing endpoints already present in the generated client (no backend changes).

  • RapidataJob.estimated_cost — approximate cost estimate for running the job to completion (job_job_id_cost_estimate_get).
  • RapidataJobDefinition.estimated_cost — same, pre-run, via the definition endpoint (job_definition_definition_id_cost_estimate_get).
  • RapidataJob.actual_cost — the billed cost, derived from the customer's billing groups (billing_costs_groups_get), matched on this job's id. None when the job isn't billed yet.

Notes

  • 409 polling. Both estimate endpoints return HTTP 409 while the job's first batch of ~5000 rapids is still being created and priced. The properties poll until the estimate becomes available (bounded by a ~5 min timeout, mirroring the SDK's existing wait/poll style) and re-raise any non-409 error immediately.
  • Accurate caveat. Docstrings state the estimate is approximate — it prices a sample of the job's created rapids and scales to the total required responses; it is not exact.
  • New thin CostEstimate / ActualCost dataclasses (re-exported from rapidata and rapidata.rapidata_client) rather than leaking the generated models.
  • New BillingService wrapper exposes the billing API on OpenAPIService, matching the existing service-accessor pattern.
  • Out of scope: RapidataOrder (order creation is deprecated; no order estimate endpoint).

Validation

  • python -c "from rapidata import RapidataClient, CostEstimate, ActualCost" → OK
  • pyright src/rapidata/rapidata_client → 0 errors
  • mkdocs build (docs) → OK; auto-generated API reference picks up the new module/properties
  • Polling logic unit-checked: retries on 409, raises on other errors, times out on persistent 409

🔗 Session: https://node-4b6e8bdf.poseidon.rapidata.internal/

Add cost properties to the SDK's job objects, backed by the existing
cost-estimate and billing endpoints in the generated client:

- RapidataJob.estimated_cost / RapidataJobDefinition.estimated_cost:
  approximate cost estimates. Both endpoints return HTTP 409 while the
  job's first batch of rapids is still being created and priced, so the
  properties poll until the estimate is available (bounded by a timeout)
  and distinguish that transient state from genuine errors.
- RapidataJob.actual_cost: the billed cost, derived by paging the
  customer's billing groups and matching the job's AudienceJob group;
  None when the job is not billed yet.

New CostEstimate / ActualCost dataclasses keep the public surface thin
instead of leaking the generated models, and are re-exported from the
package roots. A BillingService wrapper exposes the billing API on
OpenAPIService, mirroring the other service accessors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <lino@rapidata.ai>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review: feat(job): expose estimated and actual cost on jobs

Overall this is a clean, well-scoped addition that follows the repo's existing conventions closely (lazy service accessors, TYPE_CHECKING + from __future__ import annotations, tracer spans, thin dataclasses instead of leaking generated models). A few things worth a look before merging:

1. estimated_cost caches the first value forever (medium confidence)

RapidataJob.estimated_cost / RapidataJobDefinition.estimated_cost memoize into self.__estimated_cost once the poll succeeds (rapidata_job.py:174-183, rapidata_job_definition.py:44-53). But the docstring explicitly says the estimate is based on "a sample of the rapids created so far" and "the real cost can differ once every rapid has been priced" — i.e. it's meant to improve over the job's lifetime. Once cached, a user calling job.estimated_cost again later (e.g. after the job has progressed far past the first ~5000-rapid batch) will keep getting the stale, early, least-accurate number instead of a refreshed one. Contrast with get_status(), which is intentionally not cached because it changes over time — estimated_cost seems to belong in that category rather than with the immutable completed_at/pipeline_id pattern it's currently modeled after. If the memoization is intentional (e.g. to avoid repeated 409-polling costs), it'd help to say so explicitly in the docstring so users aren't surprised.

2. actual_cost does an unbounded, unfiltered, uncached scan (medium confidence)

RapidataJob.actual_cost (rapidata_job.py:186-220) paginates through all of the customer's billing groups, 100 at a time, until it finds one matching job_id or exhausts the list — and does this fresh on every property access with no memoization. billing_costs_groups_get supports start_date/end_date filters that aren't used here; passing e.g. the job's created_at as start_date would avoid scanning irrelevant history. For a customer with a large billing history this could mean many sequential HTTP round-trips per call, and repeated calls to .actual_cost multiply that cost with no caching at all (even once a job is fully billed and the result can no longer change).

3. Polling always suppresses error logging, even for unexpected errors (low confidence)

_poll_for_cost_estimate wraps the whole fetch() call in suppress_rapidata_error_logging() (cost.py:96-99) before checking whether the error is the expected 409. That's fine for the expected retry path, but it also means a genuine 4xx/5xx from the backend is fully suppressed from the SDK's normal logger.error(...) path and only surfaces as a raised exception — unlike every other API call in the SDK, which logs unexpected errors at ERROR level by default. Might be worth only suppressing logging for the 409 case (e.g. catch, check status, and only swallow-and-retry for 409, otherwise let the normal error-logging path run).

4. Docs/examples not updated

CLAUDE.md calls out "when updating any interfaces make sure you update the docs and examples." This PR adds two new public properties (estimated_cost, actual_cost) and two new public dataclasses (CostEstimate, ActualCost) but doesn't add anything to docs/ beyond what mkdocstrings will auto-pick-up from docstrings. A short mention/example (e.g. in the job/MRI guide) of how to check cost during/after a run would help discoverability, since this is a customer-facing feature.

Nits

  • The 409-polling loop can overshoot timeout by up to one interval (5s) since the deadline check happens before sleep, not after — negligible in practice given the 300s default, just noting it.
  • actual_cost's pagination relies on the default (unspecified) sort order of billing_costs_groups_get; since results aren't sorted by a stable key like created_at, if billing groups are added concurrently while paginating there's a small theoretical chance an item shifts across page boundaries mid-scan. Very unlikely to matter in practice, just flagging for completeness.

Nothing here looks blocking — mainly the caching behavior of estimated_cost (#1) and the unfiltered scan in actual_cost (#2) seem worth a second look since they affect the accuracy/performance of the feature as described in the PR itself. No test suite exists in this repo currently, so the lack of automated tests for the polling logic is consistent with existing project conventions rather than a new gap.

The billing costs-groups endpoint ranks a customer's groups by cost
descending and caps page size at 100, so an unscoped scan for a cheap
job paged through nearly the customer's entire billing history to reach
its group at the bottom of the ranking (tens of sequential requests).

Pass start_date=created_at so the backend (billable_hour >=
toStartOfHour(startDate)) drops every group older than the job. A job's
responses are only billed at or after creation, so no cost is missed.
Also terminate on items-seen rather than an assumed page size, so a
server-side page cap can't end the scan early.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <lino@rapidata.ai>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review: expose estimated/actual cost on jobs

Nice feature — clean dataclass wrapper around the generated models rather than leaking them, and the polling behavior for the 409 "not priced yet" window is a reasonable mirror of the SDK's existing wait/poll style.

What the PR does

  • Adds RapidataJob.estimated_cost / RapidataJobDefinition.estimated_cost, polling the cost-estimate endpoints (with a 409 retry loop, 5 min timeout) and caching the result once available.
  • Adds RapidataJob.actual_cost, derived by paging through billing_costs_groups_get and matching on job_id.
  • New CostEstimate / ActualCost frozen dataclasses, re-exported from rapidata and rapidata.rapidata_client.
  • New BillingService wrapper on OpenAPIService, following the existing lazy-service-accessor pattern (asset, order, flow, ...).

Potential issues

  1. actual_cost pagination relies on an unenforced sort order (rapidata_job.py, actual_cost property). The comment states the endpoint "ranks the customer's billing groups by cost descending" and uses that to justify scoping by start_date=self.created_at to bound the scan — but sort is never explicitly passed to billing_costs_groups_get. The endpoint docstring only says "ranked by cost" without specifying direction, and default sort order isn't a documented API contract. If a customer has many billing groups since the job's creation hour and this job happens to be cheap / at the "bottom" of the ranking (or the default sort isn't what's assumed), a single actual_cost access could page through a large number of billing groups (page_size=100 per round trip) before finding (or failing to find) a match. Consider passing sort= explicitly (e.g. sort=["-net_cost"] if that's the intent) so the behavior doesn't depend on an implicit backend default that could silently change.

  2. No caching on actual_cost, unlike estimated_cost, completed_at, and pipeline_id, which all memoize after the first fetch. Every access to actual_cost re-runs the full pagination scan described above. Since a billing group's cost for a job is presumably immutable once it exists (only the "not billed yet → billed" transition is a real state change), caching a resolved non-None result would avoid repeated, potentially expensive, re-scans if the caller checks this property more than once (e.g. in a polling loop waiting for billing to land).

  3. uv.lock version drift. The diff bumps rapidata from 3.15.53.15.6 in uv.lock, but the current pyproject.toml on main is already at 3.16.0. Worth rebasing and regenerating the lockfile before merge so it doesn't reintroduce a stale version pin.

Minor / non-blocking

  • estimated_cost is cached permanently after the first successful (non-409) response. Given the docstring already notes the estimate is approximate and based on a sample, this is probably fine, but worth confirming that the backend doesn't refine the estimate later in a way users might expect to observe on repeat access.
  • No unit tests were added for the new polling/pagination logic (_poll_for_cost_estimate, actual_cost's pagination loop). That said, the repo doesn't appear to have an existing test suite for rapidata_client, so this is consistent with current practice rather than a regression.
  • Docstrings are thorough and follow the Google style used elsewhere (they'll flow into the auto-generated mkdocs reference per CLAUDE.md), and lazy imports for the generated models follow the existing pattern (e.g. _get_workflow_progress). from __future__ import annotations + TYPE_CHECKING is used correctly per project convention.

Summary
Solid, well-scoped addition that follows existing SDK conventions closely. The main thing worth addressing before merge is de-risking the actual_cost pagination (either bound/sort it explicitly or cache the result) so it can't turn into an unexpectedly expensive call for customers with a lot of billing history.

Remove RapidataJob.actual_cost, the ActualCost dataclass and the
BillingService wiring - the billed-cost lookup was slow and its
value changed between reads, which is surprising for a property.

Keep the estimated_cost property (fast, stable once priced) and slim
CostEstimate to estimated_cost / datapoint_count / required_responses.
Rewrite the docstrings for first-time SDK readers, dropping internal
implementation detail.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <lino@rapidata.ai>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review

Looked at the full diff (all 3 commits squashed down to: estimated_cost on RapidataJob/RapidataJobDefinition, backed by a new job/cost.py with the CostEstimate dataclass and 409-polling helper). Overall this is clean and well-scoped — good call walking back actual_cost/BillingService in the last commit once the "value changes between reads" problem surfaced for a property.

Potential issue: unbounded-referee jobs poll for the full 5-minute timeout with a misleading error

Per the generated client's docstrings for both endpoints (job_api.py):

Returns 409 while the estimate is not yet available because [no rapids exist / the job has no rapids], or when the referee is unbounded.

_poll_for_cost_estimate (src/rapidata/rapidata_client/job/cost.py:66-91) treats every 409 as "not ready yet" and retries for up to DEFAULT_ESTIMATE_TIMEOUT (300s). But for a job/definition with an unbounded referee, the estimate can never become available — the 409 is permanent, not transient. Today that means:

  • job.estimated_cost blocks for ~5 minutes (60 polls at 5s intervals) before giving up.
  • It then raises TimeoutError("Cost estimate was not available after 300s - try again shortly."), which is actively misleading for this case — retrying will never help.

Since the response body doesn't appear to carry a distinguishing error code (at least none is referenced in the generated client), the SDK may not be able to tell the two 409 causes apart cheaply. Worth at least softening the TimeoutError message so it doesn't assert "try again shortly" unconditionally, and/or calling this caveat out in the estimated_cost docstrings (currently they only say "Raises: TimeoutError: If the estimate is still not available after a few minutes," which reads as purely transient).

Minor / non-blocking notes

  • Property vs. method for a blocking call. estimated_cost can block for up to 5 minutes and raise TimeoutError. That's a lot of hidden work/latency behind attribute access — properties are conventionally expected to be cheap and side-effect-free. A method (or at least documenting this prominently at the top of the docstring rather than just in Raises:) would set better expectations for callers who don't read the full docstring. Not a hard blocker since the codebase already has some precedent for blocking calls (e.g. wait_for_completion), but that one is explicitly a method.
  • No timeout/interval override. DEFAULT_ESTIMATE_TIMEOUT/DEFAULT_ESTIMATE_POLL_INTERVAL are hardcoded and not exposed to callers. Reasonable given the property signature can't take args, but if users hit the unbounded-referee case above, there's no way to shorten the wait — they just eat the 5 minutes every time until they stop calling it.
  • Test coverage. I don't see a unit test added for _poll_for_cost_estimate (retry-on-409 / raise-on-other-status / timeout paths), though the PR description says this was manually checked. I couldn't find an existing tests/ suite in the repo to confirm whether that's the norm here — if there is a test harness elsewhere for the polling-style helpers (e.g. around wait_for_completion), it'd be good to mirror it here since this is genuinely branchy logic (three distinct exit paths).

Things that look solid

  • CostEstimate._from_model and the dataclass fields line up correctly with GetJobCostEstimateEndpointOutput/GetJobDefinitionCostEstimateEndpointOutput (estimated_cost, datapoint_count, required_responses); dropping cost_per_response from the public surface is a reasonable simplification per the PR notes.
  • OpenAPIService.order.job_api accessor usage matches the existing lazy-service pattern (order_service.py).
  • Re-exports (job/__init__.pyrapidata_client/__init__.pyrapidata/__init__.py) are consistent and complete; no leftover ActualCost/BillingService references from the earlier commits.
  • Deadline check happens before sleep(), so no unnecessary extra wait after the timeout has already elapsed.
  • from __future__ import annotations + TYPE_CHECKING used correctly for the generated-model imports in cost.py, per the repo's stated convention.
  • No changes needed to openapi/templates/*.mustache since no generated-client files were touched.
  • Since docs are mkdocstrings-generated from docstrings (mkdocs.yml + gen_ref_pages.py), the new estimated_cost properties will show up in the API reference automatically — no manual docs page edit required here.

Nice, tightly-scoped feature. The unbounded-referee 409 case is the one thing I'd want addressed or at least explicitly called out before merge.

Immediately after a definition/job is created the estimate is not yet
priced. Besides the documented 409, the endpoint can signal this with a
success + empty body, which the generated client returns as None. That
None flowed into CostEstimate._from_model and raised AttributeError:
'NoneType' object has no attribute 'estimated_cost'.

Treat a None result as "not ready" and keep polling until the estimate
is available (or the timeout elapses), same as the 409 case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <lino@rapidata.ai>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review: feat(job): expose estimated and actual cost on jobs

Thanks for adding this — the CostEstimate polling logic is clean and the wiring through job/__init__.pyrapidata_client/__init__.pyrapidata/__init__.py follows the existing export pattern correctly.

⚠️ Scope mismatch with PR description

The PR description advertises three things: RapidataJob.estimated_cost, RapidataJobDefinition.estimated_cost, and RapidataJob.actual_cost (backed by a new ActualCost dataclass and BillingService wrapper on OpenAPIService). The actual diff (gh pr diff 658) only contains the two estimated_cost properties and the CostEstimate dataclass — there is no cost.py ActualCost class, no actual_cost property on RapidataJob, and no BillingService/billing_costs_groups_get usage anywhere in the changed files. Either a commit got dropped before pushing, or the description is stale — worth confirming before merge so reviewers are not approving based on functionality that is not actually there.

Design consideration: blocking property with a 5-minute worst case

estimated_cost polls for up to DEFAULT_ESTIMATE_TIMEOUT (300s) with sleep() inside a plain @property getter. This does match the existing style in this file (pipeline_id/completed_at also do a blocking network call from a property, and RapidataSignal already uses TimeoutError for a similar wait-with-timeout pattern — see rapidata_signal.py:260), so it is not out of line with codebase conventions, and the docstring does call out the wait. Still, going from "one HTTP call" (existing properties) to "up to a 5-minute polling loop" behind a property access is a bigger ergonomic jump than the other properties in this class. Consider whether timeout/interval should be caller-overridable (e.g. optional kwargs) rather than fixed module constants, since callers with tighter latency budgets have no way to fail fast.

Minor: cost_per_response dropped from the wrapper

Both GetJobCostEstimateEndpointOutput and GetJobDefinitionCostEstimateEndpointOutput include a cost_per_response field, but CostEstimate only surfaces estimated_cost, datapoint_count, and required_responses. If that is an intentional simplification for the public dataclass that is fine, but it is the kind of thing worth calling out explicitly in the PR notes since it is information the backend already computed and is silently discarded.

Minor: verify the "empty body to None" branch is reachable

_poll_for_cost_estimate's docstring says the "not ready" signal can be "a success with an empty body (which the generated client returns as None)", and the code branches on result is not None to handle it. However, both generated endpoint methods (job_job_id_cost_estimate_get, job_definition_definition_id_cost_estimate_get) are typed to return the model directly, not Optional[...]. If this is real runtime behavior of the generated pydantic client for a 200 with an empty body, it is worth a one-line comment noting that the type stub does not reflect it (so future readers do not assume the None branch is dead code); if it is not actually reachable, the branch can probably be simplified away.

Nice touches

  • is None cache check on self.__estimated_cost (vs. the falsy if not self.__pipeline_id a few lines above) is more correct — an estimate of 0.0 will not spuriously look "unset."
  • frozen=True dataclass with a proper docstring, TYPE_CHECKING-gated imports of the generated models, and tracer.start_as_current_span(...) spans match the SDK's existing observability conventions.
  • Failure path is safe: a TimeoutError leaves self.__estimated_cost as None, so a later access retries instead of caching a permanent failure.

Test coverage

No tests were added, but the repo does not have a test suite at all currently, so this is not a regression relative to existing conventions — just flagging since the PR description mentions the polling logic was "unit-checked," which is not reflected in any committed test file.

Nit

Per CLAUDE.md, please confirm pyright src/rapidata/rapidata_client and black src/rapidata/rapidata_client were run against the final state that will actually land (especially once the actual_cost/BillingService scope question above is resolved).

Document estimated_cost on job definitions and jobs: how to read it,
the CostEstimate fields, and the caveat that it is an estimate rather
than the final bill. Register the page in the Guides nav.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <lino@rapidata.ai>
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review

Overall this is a clean, well-scoped addition — thin CostEstimate dataclass, polling logic isolated in cost.py, and the commit history shows good iteration (dropping actual_cost once it turned out to be slow/inconsistent, and fixing the empty-body 409 edge case). Docs and exports look consistent with CLAUDE.md conventions.

One correctness issue worth fixing before merge, plus a couple of minor/optional notes:

1. RapidataJobDefinition.estimated_cost cache goes stale after update_dataset()

In rapidata_job_definition.py, estimated_cost is memoized in self.__estimated_cost and never reset:

def __init__(self, ...):
    ...
    self.__estimated_cost: CostEstimate | None = None

@property
def estimated_cost(self) -> CostEstimate:
    if self.__estimated_cost is None:
        ...
        self.__estimated_cost = CostEstimate._from_model(model)
    return self.__estimated_cost

But update_dataset() replaces the dataset with a brand new one and posts a new revision (job_definition_definition_id_revision_post). The cost-estimate endpoint docstring says it prices "the latest revision", so a fresh call after update_dataset() would legitimately return different numbers (different datapoint_count, required_responses, estimated_cost).

Repro:

job_definition = client.job.create_compare_job_definition(...)
_ = job_definition.estimated_cost              # caches estimate for revision A
job_definition.update_dataset(new_datapoints)  # revision B, totally different dataset
job_definition.estimated_cost                  # returns the stale revision-A estimate, silently

This fails silently (no exception, just wrong numbers), which is worse than the timeout/poll failure modes that were clearly designed for elsewhere in this PR. Suggest resetting self.__estimated_cost = None at the end of update_dataset().

2. Blocking property with only debug-level feedback

estimated_cost can block synchronously for up to DEFAULT_ESTIMATE_TIMEOUT (300s), and the only progress signal is logger.debug("Cost estimate not ready yet, polling..."). The default log level is WARNING (LoggingConfig.level), so in the common case a caller accessing this property sees nothing for up to 5 minutes before either a value or a TimeoutError. The new docs page also undersells this ("blocks for a moment until it is ready") — worth either bumping that log line to info/a managed_print (matching how _regenerate_results / display_progress_bar surface long waits elsewhere in this file), or adjusting the docs wording to set expectations for the worst case.

Minor, and not unprecedented — _wait_for_status in the same file has the same debug-only pattern — but a property read blocking for minutes is more surprising to a caller than an explicit get_results() call.

3. No automated tests for the polling/error branching

_poll_for_cost_estimate has several distinct paths (409 -> retry, other RapidataError -> re-raise, None body -> retry, timeout -> raise) that would be cheap to cover with a few unit tests using a fake fetch callable. The PR description mentions this was "unit-checked" manually; since this logic is easy to regress silently (for example, a future edit that widens except RapidataError to except Exception, silently swallowing real errors), it would be worth locking in with actual tests even though the repo does not have an existing test suite to extend.

Nits

  • Good catch in the second commit scoping the billing scan by start_date — glad that discipline carried through even though actual_cost itself was dropped in the final version.
  • API exports, mkdocs.yml nav entries, and the CostEstimate dataclass shape all look consistent with the rest of the SDK.

@LinoGiger LinoGiger marked this pull request as ready for review July 8, 2026 16:05
@LinoGiger LinoGiger self-requested a review as a code owner July 8, 2026 16:05
@LinoGiger LinoGiger merged commit 0ed2102 into main Jul 8, 2026
2 checks passed
@LinoGiger LinoGiger deleted the feat(job)/expose-cost-estimate-and-actual-cost branch July 8, 2026 16:05
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.

2 participants