Conversation
![spec-sync[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The PR explicitly requires human review before merging despite containing only expected mechanical spec-sync changes.
Pull request overview
Updates the mirrored V2 OpenAPI snapshot and regenerated reference models to track upstream spreadsheet-grounding changes.
Changes:
- Adds spreadsheet addresses and nullable page/box grounding.
- Adds sheet nodes and opaque element IDs.
- Bumps the mirrored API version to 1.1.0.
File summaries
| File | Description |
|---|---|
specs/v2-aide.json |
Updates the upstream V2 specification snapshot. |
specs/_generated/v2_models.py |
Regenerates reference models from the snapshot. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
yzld2002
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The PDF contract test permits invalid sheet nodes, and several grounding descriptions contradict the embedded-image box behavior.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
src/landingai_ade/types/v2/parse_response.py:160
- The new sheet fixture gives the sheet node itself
grounding.address == "Sales", so saying onlyrangeis meaningful contradicts the supported response shape. Documentaddresshere as the sheet locator as well.
# The node's spatial data. On a `page` node `box` is the full page
# `{0, 0, 1, 1}` and `page` is the 1-indexed page number; on a `sheet` node both
# are omitted (a sheet has no page number or visual position) and only `range`
# is meaningful.
tests/contract/test_v2_smoke.py:193
- Because the fixture is a PDF, accepting
"sheet"here means an invalid source-kind mapping can pass: the conditional then skips every page-specific assertion. Require"page"for this fixture; permissive deserialization of future node types is already provided by the model and should not weaken this contract check.
assert page.type in ("page", "sheet")
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Balanced
|
|
||
| - <code><a href="./src/landingai_ade/types/v2/job.py">Job</a></code> -- unified job shape: `job_id`, `status` (<code><a href="./src/landingai_ade/types/v2/job.py">JobStatus</a></code>: `pending` / `processing` / `completed` / `failed` / `cancelled`), `created_at`, `completed_at`, `progress`, `result` (a `V2ParseResponse` for parse jobs, a `V2ExtractResult` for extract jobs, a `V2BuildSchemaResponse` for build-schema jobs, or `None` until completion), `error` (<code><a href="./src/landingai_ade/types/v2/job.py">JobError</a></code>), `metadata` (the result's metadata receipt as a `dict`, populated top-level only when `output_save_url` was set and the result was delivered to `output_url` instead of inline; `None` otherwise, since inline jobs carry it on `result.metadata`), `raw` (the full original envelope as a `dict`), and the `.is_terminal` property. | ||
| - <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseResponse</a></code> -- `markdown`, `structure`, `grounding`, `metadata` (<code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseMetadata</a></code>, which nests <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseBilling</a></code> and carries `output_markdown_chars`, `range_units`, and `openapi_spec`). `structure` is a typed <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseStructure</a></code> tree (`document` → <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParsePage</a></code> → <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseElement</a></code>); each node below the root carries its spatial data inline in a <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseNodeGrounding</a></code> (`page`, <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseRange</a></code>, <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseBox</a></code>, normalized page coordinates, and an optional `confidence` in `[0, 1]` that is present only on word-granularity `atomic_grounding` segments (`dpt-3-verity`), where it is the lowest per-character OCR confidence in the word, and that is `None` on node-level grounding and on line-granularity models (`dpt-3-pro`)), and leaf elements additionally carry an `atomic_grounding` list. With `options.inline_markdown`, each node also carries its `markdown` slice. The legacy top-level `grounding` tree (<code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseGrounding</a></code> → `V2ParseGroundingPage` → `V2ParseGroundingElement` → `V2ParseGroundingEntry`) is retained for older gateway responses. Element `type`/page `status` are permissive strings and unknown keys are retained. | ||
| - <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseResponse</a></code> -- `markdown`, `structure`, `grounding`, `metadata` (<code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseMetadata</a></code>, which nests <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseBilling</a></code> and carries `output_markdown_chars`, `range_units`, and `openapi_spec`). `structure` is a typed <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseStructure</a></code> tree (`document` → <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParsePage</a></code> → <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseElement</a></code>); each node below the root carries its spatial data inline in a <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseNodeGrounding</a></code> (`page`, <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseRange</a></code>, <code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseBox</a></code>, normalized page coordinates, and an optional `confidence` in `[0, 1]` that is present only on word-granularity `atomic_grounding` segments (`dpt-3-verity`), where it is the lowest per-character OCR confidence in the word, and that is `None` on node-level grounding and on line-granularity models (`dpt-3-pro`)), and leaf elements additionally carry an `atomic_grounding` list. A `V2ParsePage` node is a `page` of a parsed document or a `sheet` of a parsed spreadsheet (`type`); a `sheet` node names itself in `id` (`None` on a `page` node). For spreadsheet content, `V2ParseNodeGrounding.page` and `.box` are `None` -- a workbook has no page number and no visual position -- and the Excel-style `address` (`Sales!C5`, `Sales!C5:F20`) locates the content instead; `address` is `None` for page-based documents. `V2ParseElement.id` is an opaque string: do not parse it or assume a format, and prefer `grounding.address`, which is stable across re-parses of the same spreadsheet. With `options.inline_markdown`, each node also carries its `markdown` slice. The legacy top-level `grounding` tree (<code><a href="./src/landingai_ade/types/v2/parse_response.py">V2ParseGrounding</a></code> → `V2ParseGroundingPage` → `V2ParseGroundingElement` → `V2ParseGroundingEntry`) is retained for older gateway responses. Element `type`/page `status` are permissive strings and unknown keys are retained. |
Comment on lines
38
to
+40
| - `box` (`V2ParseBox`) -- `{xmin, ymin, xmax, ymax}` as `[0, 1]` fractions of | ||
| the page width/height (a page node's box is the full page `{0, 0, 1, 1}`). | ||
| `None` for spreadsheet content. |
Comment on lines
+74
to
+77
| """Where a node lives: its 1-indexed `page`, its `range` in `markdown`, and its `box`. | ||
|
|
||
| Spreadsheet sources have no pages and no visual layout, so `page` and `box` are | ||
| both omitted there and the Excel-style `address` locates the content instead.""" |
Comment on lines
+163
to
+166
| # The fixture here is a PDF, so only the page-based half is assertable, and it is | ||
| # a spec guarantee rather than an environment property: `id` and `address` are | ||
| # documented as spreadsheet-only, so they are absent on every node of this | ||
| # response, while `page` and `box` are present. The spreadsheet half needs a |
Member
Author
|
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated V2 spec-sync PR (
client.v2).Gates (surface-lock, V2 contract tests, lint/test/typecheck) must pass. When present, the AI commit is a draft a human finishes (the V2 ergonomic layer — unified Job, dual-host, schema coercion — is not in the spec). Human review required before merge.
What changed
AI-generated from the PR diff — verify against the actual changes.
This PR extends the parse response model to represent spreadsheet content via new
sheetnodes and anaddressfield, alongside page-based documents.Changes:
V2ParsePage.typenow accepts"sheet"in addition to"page", and adds a newidfield holding the sheet name (Nonefor page nodes).V2ParseNodeGroundinggains a newaddressfield with an Excel-style cell/range reference, populated only for spreadsheet content.V2ParseNodeGrounding.pageand.boxare now documented and typed as nullable, returningNonefor spreadsheet content since sheets have no page number or visual position.V2ParseElement.idis now documented as an opaque string with no guaranteed format, replacing the previous<type>-<index>format documentation.