Skip to content

test: cost on harness#5075

Open
TejasGhatte wants to merge 1 commit into
devfrom
07-09-test_cost_on_harness
Open

test: cost on harness#5075
TejasGhatte wants to merge 1 commit into
devfrom
07-09-test_cost_on_harness

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a report-only logs-API cost audit to the newman-reporter-dbverify Newman reporter. After each successful billable inference response, the reporter fetches the corresponding log row via GET /api/logs/{request-id} and records whether cost was populated. This audit requires no direct database access — it uses the same API a UI would — and never fails the test run; results surface as WARN entries in the summary.

Changes

  • Added isBillableInferenceURL() to classify inference endpoints (chat completions, messages, responses, Gemini generateContent, Cohere, embeddings, audio, images) as billable while excluding utility endpoints (token counting, model/file/batch management).
  • Added verifyInferenceCostViaAPI() which polls GET /api/logs/{reqId} with exponential-ish back-off (~7.2s total) to account for asynchronous log writes and deferred cost population. Auth errors (401/403) and missing log rows (404 after all retries) surface as WARN, not FAIL.
  • Added classifyLogCost() to evaluate a fetched log row and emit PASS (cost > 0) or WARN (cost missing/zero).
  • Integrated the audit into the reporter's per-request event handler: triggered only for 2xx responses on billable inference paths, using the x-request-id response header to correlate the request to its log row.
  • Updated printSummary() to count and display WARN results alongside pass/fail/skip, with a dedicated console note when any cost checks warn.
  • Exported isBillableInferenceURL and classifyLogCost for unit testing.
  • Added index.test.js with framework-free unit tests covering billable/non-billable URL classification and cost classification logic.
  • Moved pg and better-sqlite3 to optionalDependencies so environments without native build tooling can still install the package.
  • Added a test script to package.json that runs both lib/pricing.test.js and newman-reporter-dbverify/index.test.js.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

# Run the unit tests directly
cd tests/e2e/api
npm install
npm test

# Or run just the reporter unit tests
cd tests/e2e/api/newman-reporter-dbverify
node index.test.js

Environment variables:

Variable Default Description
BIFROST_LOGS_API_COST 1 (enabled) Set to 0, false, no, or off to disable the cost audit
BIFROST_LOGS_API_AUTH (empty) Authorization header value for the logs API if dashboard auth is enabled

When the audit runs, each inference request will produce a PASS (cost populated) or WARN (cost missing or fetch error) entry in the Newman reporter summary. The run will never be failed by this check.

Breaking changes

  • Yes
  • No

Related issues

Security considerations

The BIFROST_LOGS_API_AUTH value is passed as an Authorization header to the logs API. It should be treated as a secret and not logged. The reporter already avoids printing it; ensure it is supplied via environment variable rather than committed configuration.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a report-only "Logs-API cost audit" to the Newman dbverify reporter that classifies billable inference requests, polls the logs API for populated cost, and records PASS/WARN results. Adds configuration flags, summary reporting for WARN counts, exported helper functions, unit tests, and npm test script wiring.

Changes

Logs-API Cost Audit

Layer / File(s) Summary
Cost audit implementation, config, and wiring
tests/e2e/api/newman-reporter-dbverify/index.js
Adds billable-URL detection, log-cost classification, polling GET against /api/logs/{requestId}, PASS/WARN result recording, WARN counting/reporting in the summary, logsApiCostEnabled/logsApiAuth config, request-handler wiring, and helper exports.
Unit tests for helper functions
tests/e2e/api/newman-reporter-dbverify/index.test.js
Adds assertion-based tests for isBillableInferenceURL and classifyLogCost covering billable/non-billable URLs and PASS/WARN cost outcomes.
Package.json test script wiring
tests/e2e/api/newman-reporter-dbverify/package.json, tests/e2e/api/package.json
Adds test npm scripts running the new tests and reorders dependency placement.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Newman as Newman Reporter
  participant API as Inference API
  participant LogsAPI as Logs API

  Newman->>API: send request
  API-->>Newman: 2xx response
  Newman->>Newman: isBillableInferenceURL(url)
  alt billable
    Newman->>LogsAPI: GET /api/logs/{requestId}
    LogsAPI-->>Newman: log row (cost pending)
    Newman->>LogsAPI: poll GET /api/logs/{requestId}
    LogsAPI-->>Newman: log row with cost
    Newman->>Newman: classifyLogCost(log) => PASS/WARN
  end
Loading

Possibly related PRs

  • maximhq/bifrost#4983: Both PRs update e2e cost validation logic to poll /api/logs entries until cost is populated before reporting results.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR does not address linked issue #123; it adds logs-API cost-audit tests instead of File API support. Implement the requested File API support for POST /v1/files, including the provider upload flow and related integration work.
Out of Scope Changes check ⚠️ Warning The entire changeset is unrelated to the File API support request and appears to be out of scope. Remove or split the logs-API cost-audit work into a separate PR and keep this one focused on File API support.
Title check ❓ Inconclusive The title is relevant, but it's too vague to clearly convey the main change. Use a concise title that names the main feature, such as the Newman reporter logs-API cost audit.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The description covers the template well, including summary, changes, testing, security, and checklist items.
✨ 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 07-09-test_cost_on_harness

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

@TejasGhatte TejasGhatte marked this pull request as ready for review July 9, 2026 16:36

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The e2e DB verification path can be skipped when optional DB drivers are not installed.

  • The logs API cost check matches the inspected single-log response shape and auth header contract.
  • Current inference audit paths do not overlap with existing DB verification URL mappings.
  • The DB driver dependency change can turn a required setup failure into skipped DB checks.

tests/e2e/api/package.json

Important Files Changed

Filename Overview
tests/e2e/api/newman-reporter-dbverify/index.js Adds a default-on, report-only logs API cost audit and WARN reporting for inference cost checks.
tests/e2e/api/newman-reporter-dbverify/index.test.js Adds direct Node assert coverage for the new URL classification and cost classification helpers.
tests/e2e/api/newman-reporter-dbverify/package.json Adds a local test script for the reporter helper tests.
tests/e2e/api/package.json Adds an aggregate test script and moves DB drivers into optional dependencies, which can disable DB checks when those drivers are omitted.
tests/e2e/api/package-lock.json Adds a lockfile that reflects the new local reporter dependency and optional DB drivers.

Reviews (1): Last reviewed commit: "test: cost on harness" | Re-trigger Greptile

Comment on lines +12 to +14
"optionalDependencies": {
"pg": "8.13.0",
"better-sqlite3": "11.0.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Optional Drivers Skip Checks

When better-sqlite3 or pg fails to install as an optional dependency, the DB verifier still starts, catches the missing driver, and records DB-backed checks as skipped instead of failing setup. A minimal CI image or unsupported native-addon platform can then pass the Newman run while the requested DB verification never runs.

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