Skip to content

fix(v2): remove stale credit_usage and billing char-count fields from extract metadata - #158

Closed
mikesoennichsen wants to merge 3 commits into
mainfrom
fix/v2-extract-metadata-stale-fields
Closed

fix(v2): remove stale credit_usage and billing char-count fields from extract metadata#158
mikesoennichsen wants to merge 3 commits into
mainfrom
fix/v2-extract-metadata-stale-fields

Conversation

@mikesoennichsen

Copy link
Copy Markdown
Contributor

What changed

POST /v2/extract (and the completed extract_jobs result) never sends
metadata.credit_usage or billing.input_markdown_chars /
billing.output_extraction_chars. The SDK types declared them with
defaults anyway. Pydantic filled in those defaults on model_dump(), so
every response showed a fake credit_usage: 0.0 next to the real
billing.total_credits, and a fake null for the two billing char counts
next to the real top-level values on metadata.

This removes both from V2ExtractBilling and V2ExtractMetadata. The real
char counts already live on metadata.input_markdown_chars /
metadata.output_extraction_chars and are untouched.

Why hand-edited, not regenerated

ade-python is fully hand-maintained (Stainless exit, see
docs/design/stainless-exit-and-v2.md). src/landingai_ade/types/v2/ is
hand-written SDK code, not generated output — specs/_generated/v2_models.py
is a reference-only snapshot used by the spec-sync pipeline for review, and
it already matches the fixed shape (only service_tier / total_credits on
billing, no credit_usage on metadata), confirming this is a drift the
pipeline had not caught yet.

Breaking change

Removing a field from a released type is a breaking change per this repo's
surface-lock check (griffe, gated on the last release tag). I did not use
a deprecation cycle: both removed fields always returned fake data
(0.0 / null), so there is no real value for a caller to keep reading. A
deprecation window would only keep shipping that fake data for longer. PR
carries the breaking-change-approved label.

I grepped the SDK for metadata.credit_usage reads — there are none, so no
internal migration was needed.

Verified

  • Added a wire-shape regression test in tests/test_v2_types.py and two in
    tests/api_resources/v2/test_extract.py (sync client.v2.extract(...)
    and the extract_jobs.get(...) normalize path), each built from a payload
    matching aide's real response shape and asserting model_dump() has no
    credit_usage key and no billing.*_chars keys.
  • Updated two existing tests that asserted the old, wrong shape
    (billing.input_markdown_chars, metadata.credit_usage).
  • Ran the full suite: 651 passed, 240 skipped (skips are live e2e/contract
    tests that need staging credentials).
  • ruff check, ruff format --check, mypy .: clean.
  • griffe check landingai_ade --against v1.17.1: confirms the 3 expected
    breaking removals and nothing else.
  • scripts/spec-sync/check-v2-paths.sh: unaffected, still OK.

Fixes landing-ai/aide#2013

… extract metadata

The server never sends `metadata.credit_usage` or
`billing.input_markdown_chars` / `billing.output_extraction_chars` on
`/v2/extract` responses. The SDK types declared them with defaults anyway,
so pydantic materialized a fake `credit_usage: 0.0` and always-null billing
char counts on every `model_dump()`.

Removes both from `V2ExtractBilling` and `V2ExtractMetadata`. The real char
counts already live on `metadata` itself and are untouched. No code read
`metadata.credit_usage`, so no migration is needed elsewhere in the SDK.

This is a breaking change to released public surface (griffe surface-lock
flags it). The removed fields always returned a fake `0.0` / `null`, so
there is no real value to keep — a deprecation cycle would just keep
shipping the fake data. Needs the `breaking-change-approved` label.

Fixes landing-ai/aide#2013
Copilot AI balanced review requested due to automatic review settings September 8, 2026 17:28
@mikesoennichsen mikesoennichsen added the breaking-change-approved Surface-lock override: a reviewed, intentional public-surface change label Sep 8, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

The current API-contract documentation still advertises the removed credit_usage field.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Removes stale V2 extract billing fields that caused fabricated values in serialized responses.

Changes:

  • Removes obsolete credit and character-count fields from V2 extract models.
  • Adds regression coverage for synchronous and job-result paths.
  • Updates V2 testing documentation.
File summaries
File Description
src/landingai_ade/types/v2/extract_response.py Removes stale model fields.
tests/test_v2_types.py Verifies corrected serialization shape.
tests/api_resources/v2/test_extract.py Tests sync and job responses.
docs/v2-testing.md Documents the corrected response fields.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/v2-testing.md
Comment on lines +113 to +116
— the server never sends `metadata.credit_usage` or `billing.*_chars`, so
neither field exists on the SDK types anymore (removed in the fix for
[aide#2013](https://github.com/landing-ai/aide/issues/2013); they used to
show a fake `credit_usage: 0.0` and always-null `billing.*_chars`).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — marked that table row superseded and pointed to docs/v2-testing.md for the corrected shape (commit 98e68ef).

@mikesoennichsen
mikesoennichsen requested a balanced review from Copilot September 8, 2026 17:29
…t table

Copilot review on #158 flagged that the extract-response contract table
still listed credit_usage, contradicting the fix. Marks it superseded and
points to the corrected shape.

Copilot AI left a comment

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.

🟡 Changes recommended

The updated route documentation still identifies the current wire field as version instead of model_version.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread docs/design/stainless-exit-and-v2.md Outdated
| Route | Notes |
|---|---|
| `POST /v2/extract` | Sync. JSON body: `schema` (JSON Schema, required); markdown from exactly one of `markdown` (inline) \| `markdown_ref` (from `POST /v1/files`) \| `markdown_url`; `model`; `options: {strict}`. Returns `{extraction, extraction_metadata (per-field {value, spans}), markdown, metadata {job_id, version, duration_ms, doc_id?, credit_usage}}`. **206** = partial success under strict mode. |
| `POST /v2/extract` | Sync. JSON body: `schema` (JSON Schema, required); markdown from exactly one of `markdown` (inline) \| `markdown_ref` (from `POST /v1/files`) \| `markdown_url`; `model`; `options: {strict}`. Returns `{extraction, extraction_metadata (per-field {value, spans}), markdown, metadata {job_id, version, duration_ms, doc_id?, billing}}`. Superseded: this table originally listed `credit_usage` here; the server never sent it, and the SDK's stale copy of that field was removed in [aide#2013](https://github.com/landing-ai/aide/issues/2013) — see `docs/v2-testing.md` for the current shape. **206** = partial success under strict mode. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — swapped version for model_version in that row (commit 539239b).

@mikesoennichsen
mikesoennichsen requested a balanced review from Copilot September 8, 2026 17:31
Second Copilot round on #158: the row still said version instead of
model_version, which is the field the server actually sends.

Copilot AI left a comment

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.

🟢 Approval recommended

The focused breaking change matches the documented wire contract and is covered across relevant response paths.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

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.

🟢 Approval recommended

The intentional breaking correction is consistently implemented, documented, and covered across both response paths.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@mikesoennichsen

Copy link
Copy Markdown
Contributor Author

Closing. This release cycle is scoped to the aide backend. The server wire shape is already correct (see landing-ai/aide#2013), so this SDK-side cleanup is for the SDK owners to pick up. The diff here is a working reference.

@mikesoennichsen
mikesoennichsen deleted the fix/v2-extract-metadata-stale-fields branch September 8, 2026 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change-approved Surface-lock override: a reviewed, intentional public-surface change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants