Skip to content

feat(intake): accept chat completion cost metrics#73

Merged
asutermo merged 4 commits into
mainfrom
asutermorris/update-chat-completions-endpoint
May 27, 2026
Merged

feat(intake): accept chat completion cost metrics#73
asutermo merged 4 commits into
mainfrom
asutermorris/update-chat-completions-endpoint

Conversation

@asutermo
Copy link
Copy Markdown
Contributor

@asutermo asutermo commented May 27, 2026

Summary by CodeRabbit

  • New Features

    • Added estimated USD cost fields (total, input, output) and a normalized cost breakdown map; CLI options to submit them.
    • Cost fields are recorded on ingest spans for downstream tracking.
  • Bug Fixes / Validation

    • Enforced non-negative values for all cost fields and reject deprecated producer alias fields.
    • Token counters now derive input/output totals from reported usage.
  • Tests

    • Added integration tests for persistence, validation, and usage-shape handling.

Review Change Stack

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Documentation preview is ready

Preview: https://nvidia-nemo.github.io/nemo-platform/pr-preview/pr-73/pr-73/

Built from f432172 in workflow run.

This preview is deployed from this PR branch, updates when docs changes are pushed, and will be removed when the PR closes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

Suite Lines Covered Line Rate Branch Rate
Unit Tests 18246/24195 75.4% 61.9%
Integration Tests 11668/22977 50.8% 25.9%

asutermo added 3 commits May 27, 2026 12:25
Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>
Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>
Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>
@asutermo asutermo force-pushed the asutermorris/update-chat-completions-endpoint branch from 4dc519d to cda365b Compare May 27, 2026 19:07
@asutermo asutermo marked this pull request as ready for review May 27, 2026 20:16
@asutermo asutermo requested review from a team as code owners May 27, 2026 20:16
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cc7b98bf-0bcc-4020-a909-b9687b890902

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc712e and f432172.

📒 Files selected for processing (2)
  • services/intake/src/nmp/intake/spans/ingest/chat_completions.py
  • services/intake/tests/integration/spans/test_chat_completions_ingest.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • services/intake/tests/integration/spans/test_chat_completions_ingest.py
  • services/intake/src/nmp/intake/spans/ingest/chat_completions.py

📝 Walkthrough

Walkthrough

Adds optional USD cost fields and a typed cost_details map to OpenAPI, CLI, ingest request model, and span extraction; backend computes semantic cost attributes from explicit fields or cost_details aliases; integration tests cover persistence, alias rejection, and alternative usage shapes.

Changes

Cost field support for chat completions ingest

Layer / File(s) Summary
OpenAPI cost field schema
openapi/ga/individual/platform.openapi.yaml, openapi/ga/openapi.yaml, openapi/openapi.yaml
Three OpenAPI manifests updated with numeric cost_usd, cost_input_usd, cost_output_usd (all minimum 0.0) and cost_details object allowing additional non-negative numeric breakdown fields.
CLI cost parameters for intake command
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/intake/ingest/chat_completions.py
CLI command create_chat_completions adds four optional flags: --cost-details, --cost-input-usd, --cost-output-usd, and --cost-usd, wiring them into the outgoing request payload.
Ingest request model and type constraints
services/intake/src/nmp/intake/spans/ingest/chat_completions.py
Adds Decimal/InvalidOperation and typing imports, defines NonNegativeFloat type alias, and extends ChatCompletionsIngestRequest with optional cost_usd, cost_input_usd, cost_output_usd and cost_details: dict[str, NonNegativeFloat].
Token parsing and cost extraction
services/intake/src/nmp/intake/spans/ingest/chat_completions.py
Updates attribute-building to compute input_tokens, output_tokens, total_tokens, emits cost_total_usd/cost_input_usd/cost_output_usd from request fields, and records each cost_details entry as llm.cost.{key}.
Decimal conversion helper
services/intake/src/nmp/intake/spans/ingest/chat_completions.py
Adds _decimal_or_none helper to safely convert optional float inputs to Decimal.
Integration tests for cost field ingest
services/intake/tests/integration/spans/test_chat_completions_ingest.py
Adds Decimal import and tests: persistence of producer cost fields to spans, rejection of reserved alias cost_total_usd (422), and Anthropic/Bedrock-style usage handling that avoids populating OpenAI-style token counters.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% 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 Title accurately reflects the main change: adding cost field support to chat completion intake handling across OpenAPI schemas, CLI, and service layer.
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 asutermorris/update-chat-completions-endpoint

Comment @coderabbitai help to get the list of available commands and usage tips.

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 `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 13367-13373: Update the OpenAPI schema for the cost_details object
(symbol: cost_details) to document that certain keys are reserved and will be
rejected by the backend; specifically add to the description (or an adjacent
vendorExt) a short note that the fields cost_total_usd, cost_input_usd, and
cost_output_usd are reserved and cannot be provided in cost_details (requests
including them will result in a 422 validation error), so clients know to avoid
those names.
🪄 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: CHILL

Plan: Enterprise

Run ID: 0b02536f-091a-4d7f-9cba-e1f0da5d09c8

📥 Commits

Reviewing files that changed from the base of the PR and between ba0bbfc and cda365b.

⛔ Files ignored due to path filters (5)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/cli/commands/api/intake/ingest/chat_completions.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/intake/ingest/chat_completions.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/intake/ingest/chat_completion_create_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/intake/ingest/test_chat_completions.py is excluded by !sdk/**
📒 Files selected for processing (6)
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/api/intake/ingest/chat_completions.py
  • services/intake/src/nmp/intake/spans/ingest/chat_completions.py
  • services/intake/tests/integration/spans/test_chat_completions_ingest.py

Comment thread openapi/ga/individual/platform.openapi.yaml
Comment thread services/intake/src/nmp/intake/spans/ingest/chat_completions.py Outdated
Comment thread services/intake/src/nmp/intake/spans/ingest/chat_completions.py Outdated
Comment thread services/intake/src/nmp/intake/spans/ingest/chat_completions.py Outdated
Comment thread services/intake/tests/integration/spans/test_chat_completions_ingest.py Outdated
Copy link
Copy Markdown
Contributor

@BrianNewsom BrianNewsom left a comment

Choose a reason for hiding this comment

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

Looks good after the changes we discussed live.

@asutermo asutermo force-pushed the asutermorris/update-chat-completions-endpoint branch 2 times, most recently from 1e584ba to 5fc712e Compare May 27, 2026 21:10
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 `@services/intake/src/nmp/intake/spans/ingest/chat_completions.py`:
- Around line 187-188: The loop writing body.cost_details into attributes can
overwrite canonical cost fields (e.g., total/input/output colliding with
cost_usd/cost_input_usd); change the logic in the block where
attribute_bags.put_unhandled_source_attribute is called for body.cost_details so
that you either (a) map known keys ("total","input","output") to the canonical
names ("llm.cost_usd","llm.cost_input_usd","llm.cost_output_usd") and write
those only if the canonical attribute is not already present, or (b) namespace
the incoming keys (e.g., "llm.cost_details.{key}") to avoid collisions
altogether; update the code paths around body.cost_details and
attribute_bags.put_unhandled_source_attribute accordingly so existing canonical
cost attributes are preserved.
🪄 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: CHILL

Plan: Enterprise

Run ID: aecf7064-395c-4862-bd52-43cc93f1a5b1

📥 Commits

Reviewing files that changed from the base of the PR and between 1e584ba and 5fc712e.

📒 Files selected for processing (2)
  • services/intake/src/nmp/intake/spans/ingest/chat_completions.py
  • services/intake/tests/integration/spans/test_chat_completions_ingest.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • services/intake/tests/integration/spans/test_chat_completions_ingest.py

Comment thread services/intake/src/nmp/intake/spans/ingest/chat_completions.py
Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>
@asutermo asutermo force-pushed the asutermorris/update-chat-completions-endpoint branch from 5fc712e to f432172 Compare May 27, 2026 21:53
@asutermo asutermo added this pull request to the merge queue May 27, 2026
Merged via the queue into main with commit 9d7c514 May 27, 2026
20 checks passed
@asutermo asutermo deleted the asutermorris/update-chat-completions-endpoint branch May 27, 2026 22:19
aray12 pushed a commit that referenced this pull request May 28, 2026
* feat(intake): accept chat completion cost metrics

Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>

* fix(intake): align chat completion cost fields with ATIF

Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>

* chore(intake): sync generated SDK cost fields

Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>

* fix(intake): keep chat completions usage openai-shaped

Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>

---------

Signed-off-by: Andrew Suter-Morris <asutermorris@nvidia.com>
Signed-off-by: Alex Ray <alray@nvidia.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.

2 participants